Re: [Freeipa-devel] [PATCH] [DOC] 0003 Split text commands descriptions into XML tables.
On 10/15/2013 06:19 PM, Jérôme Fenal wrote: kk2013/10/15 Martin Kosek mko...@redhat.com: Thanks. It would be ideal, if this table is (in future) generated somehow semi-automatically as practically all this info can be gathered from FreeIPA code. But for now, this is great. I see some issues with the patch though: 1) Whitespaces before tabs OK, I fixed my attached script my removing leading spaces in the second part of s///. I fixed some of them with sed -i s/\+\t$/+/g /tmp/freeipa-jfenal-0003-Split-commands-in-proper-tables.patch Bug there is the second issue: 2) Testbuild fails: $ git am /tmp/freeipa-jfenal-0003-Split-commands-in-proper-tables-1.patch Applying: Split commands in proper tables /home/mkosek/freeipa-docs/.git/rebase-apply/patch:211: space before tab in indent. row /home/mkosek/freeipa-docs/.git/rebase-apply/patch:212: space before tab in indent. entry /home/mkosek/freeipa-docs/.git/rebase-apply/patch:213: space before tab in indent. automountkey-add /home/mkosek/freeipa-docs/.git/rebase-apply/patch:214: space before tab in indent. /entry /home/mkosek/freeipa-docs/.git/rebase-apply/patch:215: space before tab in indent. entry warning: squelched 1849 whitespace errors warning: 1854 lines add whitespace errors. Will look into that. Side question, how did you get the initial text command list? It looks like `ipa help commands` output. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0075 Add ipa-advise plugins for nss-pam-ldapd legacy clients
On 10/18/2013 04:07 PM, Alexander Bokovoy wrote:
On Fri, 18 Oct 2013, Ana Krivokapic wrote:
On 10/18/2013 01:31 PM, Ana Krivokapic wrote:
On 10/18/2013 09:48 AM, Martin Kosek wrote:
On 10/17/2013 10:29 PM, Alexander Bokovoy wrote:
On Thu, 17 Oct 2013, Ana Krivokapic wrote:
Hello,
This patch adds ipa-advise plugins for configuring legacy clients
using
nss-pam-ldapd.
https://fedorahosted.org/freeipa/ticket/3672
Thanks. Looks good. I have one comment below
+class config_freebsd_nss_pam_ldapd(config_base_legacy_client):
+
+Legacy client configuration for FreeBSD, using nss-pam-ldapd.
+
+description = ('Instructions for configuring a FreeBSD system
with '
+ 'nss-pam-ldapd. ')
+
+def get_info(self):
+uri, base = self.get_uri_and_base()
+cacrt = '/usr/local/etc/ipatest.crt'
Is the cert file name is correct? 'ipatest.crt'? Perhaps 'ipaca.crt'
would be a better name?
Or simply ipa.crt since it is the filename used everywhere else...
Martin
Cert file name changed to ipa.crt.
Comment added about AES not being available on RHEL5.
Updated patch attached.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
This updated patch improves the note about possible issues regarding
encryption
algorithms.
Thanks. ACK.
Pushed to master: 92cd987e0a347123d81f83be99787ab77f39ca8e
--
Petr³
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0289-0302 Managed Read permissions
On 10/03/2013 12:42 PM, Martin Kosek wrote: On 10/02/2013 01:26 PM, Petr Viktorin wrote: On 10/02/2013 01:07 PM, Simo Sorce wrote: ... To sum it up, I would rather not build our permission system on this group. I think we need top base our ACIs on LDAP bind targets ldap:///all and ldap:///anyone to avoid performance issues and issues with ipausers not being complete: https://access.redhat.com/site/documentation/en-US/Red_Hat_Directory_Server/8.2/html/Administration_Guide/Managing_Access_Control-Bind_Rules.html I rather think we want to base the permissions on binddn. In the beginning, there would be 3 types of permissions based on binddn: * groupdn based: standard permissions that can be assigned privileges * ldap:///all permissions for all authenticated users. Cannot be assigned to privileges (would not make sense) * ldap:///anyone permissions for all, including anonymous users. Cannot be assigned to privileges (would not make sense) Just few examples: Read users - ldap:///anyone Read groups - ldap:///anyone Read sudo - ldap:///all Read hbac - ldap:///all ... Basing the permissions on these would allow us to get rid of all the awful deny permissions. To make the permission Bind part more user friendly, there should be an option in permission-find and a switch in Web UI to e.g. list permissions by bind type, i.e.: - anonymous permissions - authenticated users permissions - group based permissions If use would want to e.g restrict sudo only to specified group, I would see this workflow: 1) Change bind type from authenticated users to group based 2) Bind permission to chosen privilege 3) ... And the opposite, if he wants to make reading more open: 1) Unbind permission from privilege 2) Change bind type to authenticated users or anonymous Makes sense? Yes. I agree with Martin's comments too. Simo. We use privileges to group related permissions. For example an Automount Reader privilege would contain read automount keys and read automount maps permissions. Wouldn't it make more sense (from the user's point of view at least) to have this setting at the privilege level? The selfservice plugin does pretty much the same thing. Should we (aim to) move selfservice functionality to this system? selfservice is not involved in privileges, neither is delegation, they are just handling raw ACIs in a custom manner. With ldap:///all and ldap:///anyone we could theoretically add new permission types and commands, like anonymousrbac-add or authenticatedrbac-add, but I think it would be best to keep them with standard permissions for several reasons: 1) Can use the new cool features you are adding to permission entries 2) With these ACIs, it makes sense to convert them to group-based permission and assign to a privilege (does not make that much sense with selfservice ACIs). AFAIU, we all agree on that and the only part is how the anonymous/authenticated-user permissions should be grouped. I still think that grouping them in privileges or roles does not make much sense - anonymous/authenticated-user ACIs do not need to be grouped anywhere, setting binddn is enough. It would be just our custom wiring in framework without much benefit to real functionality in LDAP. If you have a privilege Automount Reader with setting Access: Anonymous: or similar, what if you add more non-anonymous permission to the privilege? What would happen then? Or what if you remove permission from the privilege, should it automagically become group-based ACI? Going with the workflow I described above seems to me as the most direct approach with keeping the anonymous/authenticated users/group based information in the single authoritative source - ACI/permission. And, since these permissions are longer be compatible with old versions, I could move them out of $SUFFIX and onto the relevant containers. That should also help performance. +1 Martin Alright, I'm crafting an updated design page with the above in mind. Here are the main differences. New permissions won't (necessarily) be in $SUFFIX, so old IPA servers will not be able to modify them. Extra attribute types needed in addition to ipaPerm*Attributes would be: - ipaPermBindType (anonymous/any authenticated user/normal permission) - ipaPermDN (container DN where the ACI is stored) And objectclasses to group them: 'ipaPermissionV2' SUP ipaPermission AUXILIARY MUST ( ipaPermBindType $ ipaPermDN ) 'ipaManagedPermission' SUP ipaPermissionV2 AUXILIARY MAY ( ipaPermDefaultAttrs $ ipaPermAllowedAttrs $ ipaPermExcludedAttrs ) As for 'ipaPermissionV2', all non-SYSTEM permissions should be updated to it. Maybe a better name is needed. Another idea I had is to store all variable parts of the ACI in the permission entry. This would mean we'd not need to parse ACIs to read, search, or update them, which should make these operations faster and the code could be simplified. Doing this would require these new attribute types: - ipaPermRight (add
Re: [Freeipa-devel] Reviews still needed
On 10/09/2013 08:57 PM, Nathaniel McCallum wrote: I still need reviews on the following patches. The first two (0015 and 0016) should be close if not ready to merge. They have undergone four revisions. The third is probably in the middle of reviews. Please help me push this over the goal line. :) Nathaniel freeipa-npmccallum-0015-4-Add-support-for-managing-user-auth-types.patch API.txt | 12 VERSION |2 +- install/updates/50-ipaconfig.update |1 + ipalib/plugins/config.py|8 ipalib/plugins/user.py | 35 ++-- 5 files changed, 43 insertions(+), 15 deletions(-) I'm waiting for your response on this one. Did you reproduce the test failure? Unfortunately the other two don't apply to master. Could you rebase them? freeipa-npmccallum-0016-4-Add-RADIUS-proxy-support-to-ipalib-CLI.patch API.txt| 95 +++-- VERSION|2 install/share/70ipaotp.ldif|2 install/share/indices.ldif | 10 ++ install/share/referint-conf.ldif |3 install/updates/10-70ipaotp.update |2 install/updates/20-indices.update |7 + install/updates/25-referint.update |1 install/updates/40-otp.update |5 + ipalib/constants.py|1 ipalib/plugins/config.py |2 ipalib/plugins/radiusproxy.py | 138 + ipalib/plugins/user.py | 44 ++- 13 files changed, 298 insertions(+), 14 deletions(-) freeipa-npmccallum-0024-2-Add-OTP-support-to-ipalib-CLI.patch API.txt| 101 + VERSION|2 freeipa.spec.in|2 ipalib/errors.py | 16 ++ ipalib/plugins/config.py |2 ipalib/plugins/otptoken.py | 330 + ipalib/plugins/user.py | 10 + 7 files changed, 456 insertions(+), 7 deletions(-) -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0288 Use a user result template in tests
On 10/18/2013 04:21 PM, Ana Krivokapic wrote: On 09/30/2013 05:05 PM, Petr Viktorin wrote: Hello, This patch introduces an user template with the result of a default user add/show. The template is then customized and used in each test. This makes the tests shorter, and highlights the non-default (interesting) pieces of the result instead of presenting a wall of text. Also, when a new default attribute is added to user results (as is the case in my upcoming ACI patches), there's now only one place to change. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ACK with two tiny nitpicks: +Attributes named in ``omit`` are removed from the result; any additional +or non-default values can be specified in``overrides``. ^ missing space + +# sn can be None; this should only used from `get_admin_result` ... this should only *be* used ... Thanks for your attention to detail! Fixed pushed to master: 756b997a7d2a4c23b41469ee272e412d7f8ca19f -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0274 test_simple_replication: Fix waiting for replication
On 10/18/2013 05:53 PM, Ana Krivokapic wrote: On 09/13/2013 06:24 PM, Petr Viktorin wrote: The simple replication test is failing intermittently. It's quite hard to manually verify if this patch fixes that completely, but my testing says it does make a positive difference. See commit message for details. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel The simple replication test seems to work fine with the patch applied, the code changes look sane, so ACK from me. Thanks for the review! Pushed to master: f34b8896f97ee8b29da7f167c7262071000727e4 ipa-3-3: e6738ea68f318c45ad901f8ecfe2d9d94694c7f4 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0289-0302 Managed Read permissions
On 10/21/2013 03:57 PM, Martin Kosek wrote: On 10/18/2013 04:28 PM, Petr Viktorin wrote: On 10/03/2013 12:42 PM, Martin Kosek wrote: On 10/02/2013 01:26 PM, Petr Viktorin wrote: On 10/02/2013 01:07 PM, Simo Sorce wrote: ... To sum it up, I would rather not build our permission system on this group. I think we need top base our ACIs on LDAP bind targets ldap:///all and ldap:///anyone to avoid performance issues and issues with ipausers not being complete: https://access.redhat.com/site/documentation/en-US/Red_Hat_Directory_Server/8.2/html/Administration_Guide/Managing_Access_Control-Bind_Rules.html I rather think we want to base the permissions on binddn. In the beginning, there would be 3 types of permissions based on binddn: * groupdn based: standard permissions that can be assigned privileges * ldap:///all permissions for all authenticated users. Cannot be assigned to privileges (would not make sense) * ldap:///anyone permissions for all, including anonymous users. Cannot be assigned to privileges (would not make sense) Just few examples: Read users - ldap:///anyone Read groups - ldap:///anyone Read sudo - ldap:///all Read hbac - ldap:///all ... Basing the permissions on these would allow us to get rid of all the awful deny permissions. To make the permission Bind part more user friendly, there should be an option in permission-find and a switch in Web UI to e.g. list permissions by bind type, i.e.: - anonymous permissions - authenticated users permissions - group based permissions If use would want to e.g restrict sudo only to specified group, I would see this workflow: 1) Change bind type from authenticated users to group based 2) Bind permission to chosen privilege 3) ... And the opposite, if he wants to make reading more open: 1) Unbind permission from privilege 2) Change bind type to authenticated users or anonymous Makes sense? Yes. I agree with Martin's comments too. Simo. We use privileges to group related permissions. For example an Automount Reader privilege would contain read automount keys and read automount maps permissions. Wouldn't it make more sense (from the user's point of view at least) to have this setting at the privilege level? The selfservice plugin does pretty much the same thing. Should we (aim to) move selfservice functionality to this system? selfservice is not involved in privileges, neither is delegation, they are just handling raw ACIs in a custom manner. With ldap:///all and ldap:///anyone we could theoretically add new permission types and commands, like anonymousrbac-add or authenticatedrbac-add, but I think it would be best to keep them with standard permissions for several reasons: 1) Can use the new cool features you are adding to permission entries 2) With these ACIs, it makes sense to convert them to group-based permission and assign to a privilege (does not make that much sense with selfservice ACIs). AFAIU, we all agree on that and the only part is how the anonymous/authenticated-user permissions should be grouped. I still think that grouping them in privileges or roles does not make much sense - anonymous/authenticated-user ACIs do not need to be grouped anywhere, setting binddn is enough. It would be just our custom wiring in framework without much benefit to real functionality in LDAP. If you have a privilege Automount Reader with setting Access: Anonymous: or similar, what if you add more non-anonymous permission to the privilege? What would happen then? Or what if you remove permission from the privilege, should it automagically become group-based ACI? Going with the workflow I described above seems to me as the most direct approach with keeping the anonymous/authenticated users/group based information in the single authoritative source - ACI/permission. And, since these permissions are longer be compatible with old versions, I could move them out of $SUFFIX and onto the relevant containers. That should also help performance. +1 Martin Alright, I'm crafting an updated design page with the above in mind. Here are the main differences. New permissions won't (necessarily) be in $SUFFIX, so old IPA servers will not be able to modify them. Extra attribute types needed in addition to ipaPerm*Attributes would be: - ipaPermBindType (anonymous/any authenticated user/normal permission) - ipaPermDN (container DN where the ACI is stored) And objectclasses to group them: 'ipaPermissionV2' SUP ipaPermission AUXILIARY MUST ( ipaPermBindType $ ipaPermDN ) 'ipaManagedPermission' SUP ipaPermissionV2 AUXILIARY MAY ( ipaPermDefaultAttrs $ ipaPermAllowedAttrs $ ipaPermExcludedAttrs ) As for 'ipaPermissionV2', all non-SYSTEM permissions should be updated to it. Maybe a better name is needed. Another idea I had is to store all variable parts of the ACI in the permission entry. This would mean we'd not need to parse ACIs to read, search, or update them, which should make these operations faster and the code could
Re: [Freeipa-devel] [PATCH 0121] ipatests: Add support for hosts referenced by a keyword
On 10/22/2013 09:20 AM, Tomas Babej wrote: Hi, Adds support for host definition by a environment variables of the following form: KEYWORDHOST__envX, where X is the number of the environment for which host referenced by a keyword should be defined. You can also optionally use KEYWORDHOST__IP_envX to define the IP address directly, otherwise the framework will try to resolve the hostname. Adds a required_keyword_hosts attribute to the IntegrationTest class, which can test developer use to specify the keyword hosts that this particular test requires. If not all required keyword hosts are available, the test will be skipped. All keyword hosts are accessible to the IntegrationTests via the keyword_hosts attribute, which contains a dictionary keyed by the keywords. Why is this necessary? What's wrong with just extending the current scheme with more roles? -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 100-106] Initial implementation of AD integration tests
Replying to one part only: On 10/21/2013 04:50 PM, Tomas Babej wrote: On 10/16/2013 03:44 PM, Petr Viktorin wrote: I still think it would be simpler if IPA and AD domains shared the numbering namespace (users would need to define $AD_env2; if they had $MASTER_env1 and $AD_env1 they would be in the same Domain). But if we use _env1 for both the AD and the IPA domain, they need to be separated in Domain.from_env. With patch 0101 both MASTER_env1 and AD_env1 will be in both domains[0] and ad_domains[0]. I would rather not join IPA and AD domains as they even cannot be in the same domain, as the service records would clash. So these will always be separate / sub / super domain relationship. You're right that they should never share the same domain. But you should never say never, especially in testing -- what if we'll want to, in the future, test that the records *do* clash, or that IPA refuses to install in an AD domain? Another problem is that they are now separate namespaces. In all code that deals with domains you have to deal separately with the list of AD domains and separately with IPA domains. This makes every piece of code that doesn't care much about what type of domain it's dealing with (configuration, listing, possible automation scripts for turning on the VMs, etc.) more complicated. Also, in this scheme, adding a new type of domain would be quite hard, especially after more code is written with this split in mind. Do keep the domain type, though. tl;dr I'd really prefer domain 1 (IPA); domain 2 (AD) rather than IPA domain 1; AD domain 1. If needed we can have a special check that would reject IPA masters in AD domains and vice versa, if that really turns out to be necessary. As we already pass ad_domain flag to Domain.from_env, I did incorporate code that joins the machines to the domain depending on the their role. Is that a viable solution for you? Sorry. I think this design is less sustainable than having a shared namespace for the domains. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0121] ipatests: Add support for hosts referenced by a keyword
On 10/22/2013 10:09 AM, Tomas Babej wrote: On 10/22/2013 09:54 AM, Petr Viktorin wrote: On 10/22/2013 09:20 AM, Tomas Babej wrote: Hi, Adds support for host definition by a environment variables of the following form: KEYWORDHOST__envX, where X is the number of the environment for which host referenced by a keyword should be defined. You can also optionally use KEYWORDHOST__IP_envX to define the IP address directly, otherwise the framework will try to resolve the hostname. Adds a required_keyword_hosts attribute to the IntegrationTest class, which can test developer use to specify the keyword hosts that this particular test requires. If not all required keyword hosts are available, the test will be skipped. All keyword hosts are accessible to the IntegrationTests via the keyword_hosts attribute, which contains a dictionary keyed by the keywords. Why is this necessary? What's wrong with just extending the current scheme with more roles? The need for keyword hosts arised with the implementation of the legacy client test suite. As each of these tests needs a precise type (pre-defined and pre-configured) of VM, and not a generic client, you need to restrict the test case to specific host type. One test might be restricted to RHEL 5.10 with nss-pam-ldapd, another to FreeBSD, next one to CentOS with nss-pam-ldapd, next to CentOS with old version of SSSD... Each testcase that needs a new type of preconfigured host, we would need to introduce a new role, which would need to be integrated in the framework. In such implementation, we would lose loose coupling between the test framework and the test themselves, and make them less pluggable. The framework itself (excluding the configuration part) should be able to handle this nicely using the existing role mechanism. It's true that in some places it's currently hard-wired to just a few roles, but supporting completely custom roles shouldn't be a problem. I'd prefer if this system was used, rather than basically adding a second role system, which we'd have to support even if we get rid of the current config part. The envvar-based configuration is not as flexible, but I'd rather make this part somewhat unpleasant than making the framework complex. We plan to move to a simpler configuration method anyway. That said, you can basically keep the variable name scheme you use in this patch; just maybe use TESTHOST rather than KEYWORDHOST. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCHES] 0289-0294 Fixes in permissions
Here are refactorings and fixes for small issues I found so far while
working on ticket #3566.
Having these already in master should make the final patchset easier to
review.
--
Petr³
From d08b18cdac29120159217a9d43ebe3ce80eff3b0 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Thu, 12 Sep 2013 10:43:44 +0200
Subject: [PATCH] Update Permission and ACI plugins to decorator registration
API
---
ipalib/plugins/aci.py| 24 +++-
ipalib/plugins/permission.py | 30 +-
2 files changed, 24 insertions(+), 30 deletions(-)
diff --git a/ipalib/plugins/aci.py b/ipalib/plugins/aci.py
index a7f85dd367fd7cfd200cb5b1fbd95b7e261a5290..76f87aaf881b5ef9cd7d9706722d843c6d819268 100644
--- a/ipalib/plugins/aci.py
+++ b/ipalib/plugins/aci.py
@@ -125,10 +125,13 @@
from ipalib.aci import ACI
from ipalib import output
from ipalib import _, ngettext
+from ipalib.plugable import Registry
from ipalib.plugins.baseldap import gen_pkey_only_option
from ipapython.ipa_log_manager import *
from ipapython.dn import DN
+register = Registry()
+
ACI_NAME_PREFIX_SEP = :
_type_map = {
@@ -419,6 +422,8 @@ def _normalize_permissions(perm):
values=_valid_prefix_values,
)
+
+@register()
class aci(Object):
ACI object.
@@ -501,8 +506,8 @@ class aci(Object):
),
)
-api.register(aci)
+@register()
class aci_add(crud.Create):
Create new ACI.
@@ -555,9 +560,8 @@ def execute(self, aciname, **kw):
value=aciname,
)
-api.register(aci_add)
-
+@register()
class aci_del(crud.Delete):
Delete ACI.
@@ -597,9 +601,8 @@ def execute(self, aciname, aciprefix, **options):
value=aciname,
)
-api.register(aci_del)
-
+@register()
class aci_mod(crud.Update):
Modify ACI.
@@ -666,9 +669,8 @@ def execute(self, aciname, **kw):
value=aciname,
)
-api.register(aci_mod)
-
+@register()
class aci_find(crud.Search):
Search for ACIs.
@@ -872,9 +874,8 @@ def execute(self, term, **kw):
truncated=False,
)
-api.register(aci_find)
-
+@register()
class aci_show(crud.Retrieve):
Display a single ACI given an ACI name.
@@ -914,9 +915,8 @@ def execute(self, aciname, **kw):
value=aciname,
)
-api.register(aci_show)
-
+@register()
class aci_rename(crud.Update):
Rename an ACI.
@@ -976,5 +976,3 @@ def execute(self, aciname, **kw):
result=result,
value=kw['newname'],
)
-
-api.register(aci_rename)
diff --git a/ipalib/plugins/permission.py b/ipalib/plugins/permission.py
index 11819890af26ec8bb8a4b1a4cebdbb9b67453891..2284dbe4db4eaff02100f1ffea438a34ad5fa551 100644
--- a/ipalib/plugins/permission.py
+++ b/ipalib/plugins/permission.py
@@ -20,6 +20,7 @@
from ipalib.plugins.baseldap import *
from ipalib import api, _, ngettext
from ipalib import Flag, Str, StrEnum
+from ipalib.plugable import Registry
from ipalib.request import context
from ipalib import errors
from ipapython.dn import DN, EditableDN
@@ -80,6 +81,8 @@
ACI_PREFIX=upermission
+register = Registry()
+
output_params = (
Str('ipapermissiontype',
label=_('Permission Type'),
@@ -99,6 +102,8 @@ def filter_options(options, keys):
return dict((k, options[k]) for k in keys if k in options)
+
+@register()
class permission(LDAPObject):
Permission object.
@@ -194,9 +199,8 @@ def filter_aci_attributes(self, options):
return dict((k, v) for k, v in options.items() if
k in self.aci_attributes)
-api.register(permission)
-
+@register()
class permission_add(LDAPCreate):
__doc__ = _('Add a new permission.')
@@ -251,8 +255,8 @@ def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
raise e
return dn
-api.register(permission_add)
+@register()
class permission_add_noaci(LDAPCreate):
__doc__ = _('Add a system permission without an ACI')
@@ -285,9 +289,8 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
entry_attrs['ipapermissiontype'] = [ permission_type ]
return dn
-api.register(permission_add_noaci)
-
+@register()
class permission_del(LDAPDelete):
__doc__ = _('Delete a permission.')
@@ -313,9 +316,8 @@ def pre_callback(self, ldap, dn, *keys, **options):
pass
return dn
-api.register(permission_del)
-
+@register()
class permission_mod(LDAPUpdate):
__doc__ = _('Modify a permission.')
@@ -403,9 +405,8 @@ def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
entry_attrs[r] = result[r]
return dn
-api.register(permission_mod)
-
+@register()
class permission_find(LDAPSearch):
__doc__ = _('Search for permissions.')
@@ -485,9 +486,8 @@ def post_callback(self, ldap, entries, truncated, *args, **options
Re: [Freeipa-devel] [PATCHES 100-106] Initial implementation of AD integration tests
On 10/22/2013 02:24 PM, Tomas Babej wrote: On 10/22/2013 02:15 PM, Tomas Babej wrote: On 10/22/2013 12:27 PM, Tomas Babej wrote: On 10/22/2013 10:37 AM, Petr Viktorin wrote: Replying to one part only: On 10/21/2013 04:50 PM, Tomas Babej wrote: On 10/16/2013 03:44 PM, Petr Viktorin wrote: I still think it would be simpler if IPA and AD domains shared the numbering namespace (users would need to define $AD_env2; if they had $MASTER_env1 and $AD_env1 they would be in the same Domain). But if we use _env1 for both the AD and the IPA domain, they need to be separated in Domain.from_env. With patch 0101 both MASTER_env1 and AD_env1 will be in both domains[0] and ad_domains[0]. I would rather not join IPA and AD domains as they even cannot be in the same domain, as the service records would clash. So these will always be separate / sub / super domain relationship. You're right that they should never share the same domain. But you should never say never, especially in testing -- what if we'll want to, in the future, test that the records *do* clash, or that IPA refuses to install in an AD domain? You could still set AD_env1 and MASTER_env1 to have the same domain. Another problem is that they are now separate namespaces. In all code that deals with domains you have to deal separately with the list of AD domains and separately with IPA domains. This makes every piece of code that doesn't care much about what type of domain it's dealing with (configuration, listing, possible automation scripts for turning on the VMs, etc.) more complicated. Also, in this scheme, adding a new type of domain would be quite hard, especially after more code is written with this split in mind. Do keep the domain type, though. tl;dr I'd really prefer domain 1 (IPA); domain 2 (AD) rather than IPA domain 1; AD domain 1. This will, however, require filtering the domains depending on the fact whether they contain AD or not. If a testcase wants to access an AD domain, it will still need to loop over the list of domains to see which ones are of AD type. Any code that does not care what domain type it's dealing with, can easily access all the domains by chaining the respective iterables. We could have a wrapper in the Config class for that, along the lines of get_all_domains(). So what I see here is that we're trading one complexity over another. I think we can agree on your approach since it hides the complexity from the user, especially in the ipa-test-config, which I admit is getting rather ugly, as we need to introduce new option there and that causes splitting. If needed we can have a special check that would reject IPA masters in AD domains and vice versa, if that really turns out to be necessary. With this check we should be fine. As we already pass ad_domain flag to Domain.from_env, I did incorporate code that joins the machines to the domain depending on the their role. Is that a viable solution for you? Sorry. I think this design is less sustainable than having a shared namespace for the domains. I'll send revised patchset soon. Updated patchset attached. Patch 105: Typo: 'Sets DNS ib given host for trust with the given AD ^^ Should be in, right? I'll fix it before pushing. Otherwise, these are good to go! Patch 106: In ADTrustBase, it looks like if test_install_adtrust or test_configure_dns_and_time fail, it doesn't make much sense to run the other tests. If that's the case they can go in an install() classmethod. Same with test_establish_trust* in the subclasses. Also, there's a typo in test_estabilish_trusts several times. -^ -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCHES] 0313-0314 Integration test fixes
Here are fixes for two bugs found while running integration tests under
Beaker.
--
Petr³
From c568f9c83eaf6e579efaf03fc9580ae46cf0b61c Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Thu, 24 Oct 2013 13:55:47 +0200
Subject: [PATCH] Tests: mkdir_recursive: Don't fail when top-level directory
doesn't exist
When the directory directly under root (e.g. /etc) did not exist,
mkdir_recursive failed.
Fix the issue.
---
ipatests/test_integration/transport.py | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/ipatests/test_integration/transport.py b/ipatests/test_integration/transport.py
index a0bd3700ac4e9887804f021d1a2fb4eb050457d3..9b3dd5be5fc4df7b235f52e2f5dcdb9fc608ad23 100644
--- a/ipatests/test_integration/transport.py
+++ b/ipatests/test_integration/transport.py
@@ -87,10 +87,10 @@ def start_shell(self, argv, log_stdout=True):
def mkdir_recursive(self, path):
`mkdir -p` on the remote host
-if not path or path == '/':
-raise ValueError('Invalid path')
-if not self.file_exists(path or '/'):
-self.mkdir_recursive(os.path.dirname(path))
+if not self.file_exists(path):
+parent_path = os.path.dirname(path)
+if path != parent_path:
+self.mkdir_recursive(parent_path)
self.mkdir(path)
def get_file(self, remotepath, localpath):
--
1.8.3.1
From f32b4a582974e36d63913ddefdcd29965bee5848 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Wed, 23 Oct 2013 14:05:39 +0200
Subject: [PATCH] beakerlib plugin: Don't try to submit logs if they are
missing
---
ipatests/beakerlib_plugin.py | 1 +
1 file changed, 1 insertion(+)
diff --git a/ipatests/beakerlib_plugin.py b/ipatests/beakerlib_plugin.py
index 45f34c6a6460f52fa0b5b9fb9b8e8bb2975ccafa..71c1df537fb98ae49b159bd1e80381a37dc8a51f 100644
--- a/ipatests/beakerlib_plugin.py
+++ b/ipatests/beakerlib_plugin.py
@@ -116,6 +116,7 @@ def collect_logs(self, logs_to_collect):
raiseonerr=False)
if cmd.returncode:
self.log.warn('Could not collect all requested logs')
+return
# Copy and unpack on the local side
topdirname = tempfile.mkdtemp()
--
1.8.3.1
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 433-434 Remove mod_ssl conflict
On 10/25/2013 10:31 AM, Martin Kosek wrote:
Since mod_nss-1.0.8-24, mod_nss and mod_ssl can co-exist on one
machine (of course, when listening to different ports).
To make sure that mod_ssl is not configured to listen on 443
(default mod_ssl configuration), add a check to the installer checking
of either mod_nss or mod_ssl was configured to listen on that port.
https://fedorahosted.org/freeipa/ticket/3974
TO TEST:
1. Install newest mod_nss:
F19: http://koji.fedoraproject.org/koji/buildinfo?buildID=473624
2. Install patched freeipa
3. Install mod_ssl
4. Update /etc/httpd/conf.d/ssl.conf to not listen on 443, but rather on
10443 or others
5. setenforce 0 to allow httpd listen on that port
6. ipa-server-install
When mod_ssl.rpm is instaled *after* ipa-server-install, no check is
done, Apache just fails to start.
We need to document this.
The server should now listen on both 443 with mod_nss and 10443 with
mod_ssl. CLI and Web UI should continue to work, as well as cert
operations like cert-show 1 - cert operations would not work if new
mod_nss is not updated.
That is the Apache server, right? IPA is only on 443.
Martin
freeipa-mkosek-433-make-set_directive-and-get_directive-more-strict.patch
ACK
freeipa-mkosek-434-remove-mod_ssl-conflict.patch
Just a comment on logging:
+def httpd_443_configured():
+
+We now allow mod_ssl to be installed so don't automatically disable it.
+However it can't share the same listen port as mod_nss, so check for that.
+
+Returns True if something other than mod_nss is listening on 443.
+False otherwise.
+
+try:
+(stdout, stderr, rc) = ipautil.run(['/usr/sbin/httpd', '-t', '-D',
'DUMP_VHOSTS'])
+except ipautil.CalledProcessError, e:
+print sys.stderr, WARNING: cannot check if port 443 is already
configured.
+print sys.stderr, httpd returned error when checking:, str(e)
+return False
+
+port_line_re = re.compile(r'(?Paddress\S+):(?Pport\d+)')
+for line in stdout.splitlines():
+m = port_line_re.match(line)
+if m and int(m.group('port')) == 443:
+print WARNING: Apache is already configured with a listener on port
443:
+print line
+return True
Please also log these messages, otherwise the log ends up not being very
helpful.
Since the installation aborts, I think these should be ERROR or
CRITICAL, not WARNING.
--
Petr³
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 100-106] Initial implementation of AD integration tests
On 10/24/2013 04:38 PM, Tomas Babej wrote: On 10/24/2013 01:29 PM, Petr Viktorin wrote: [...] Patch 106: In ADTrustBase, it looks like if test_install_adtrust or test_configure_dns_and_time fail, it doesn't make much sense to run the other tests. If that's the case they can go in an install() classmethod. Same with test_establish_trust* in the subclasses. I made them part of the install() classmethod. As for the test_estabilish_trust, I would have to still override that in each class that uses it, since all of them use it in a slightly different way. I'd prefer an establish_trust classmethod called from install(), but it's not really important in this case. Also, there's a typo in test_estabilish_trusts several times. -^ Typo fixed. Also attaching a patch that fixes the same type in the other parts of the codebase. ACK, ACK, pushed to master: df5f5c9fab5dd66c50bf202e1ebd19f558e3e0c6 ipa-3-3: 5dbd11722e365f50b7496b4ab2559122cd927d53 Thanks for your patience! -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0077 Do not roll back failed client installation on server
On 10/24/2013 05:48 PM, Ana Krivokapic wrote: Hello, This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3990 ACK, pushed to: master: c518a80ab7faa8cbb399e3ed32c213ad518d997c ipa-3-3: 24073d22e2e829ccba49e698c45e07b69cf25770 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 433-434 Remove mod_ssl conflict
On 10/25/2013 02:09 PM, Martin Kosek wrote: On 10/25/2013 12:33 PM, Petr Viktorin wrote: On 10/25/2013 10:31 AM, Martin Kosek wrote: Since mod_nss-1.0.8-24, mod_nss and mod_ssl can co-exist on one machine (of course, when listening to different ports). To make sure that mod_ssl is not configured to listen on 443 (default mod_ssl configuration), add a check to the installer checking of either mod_nss or mod_ssl was configured to listen on that port. https://fedorahosted.org/freeipa/ticket/3974 TO TEST: 1. Install newest mod_nss: F19: http://koji.fedoraproject.org/koji/buildinfo?buildID=473624 2. Install patched freeipa 3. Install mod_ssl 4. Update /etc/httpd/conf.d/ssl.conf to not listen on 443, but rather on 10443 or others 5. setenforce 0 to allow httpd listen on that port 6. ipa-server-install Okay, I found another problem. After the above steps: - ipa-server-install --uninstall - Uninstall mod_ssl - ipa-server-install When mod_ssl.rpm is instaled *after* ipa-server-install, no check is done, Apache just fails to start. We need to document this. Document where exactly? Ideas welcome. FreeIPA server uses set of ports, defined in http://docs.fedoraproject.org/en-US/Fedora/18/html/FreeIPA_Guide/installing-ipa.html#prerequisites Well, at least in the release notes. The guide you linked to could also have note that this conflicts with the mod_nss defaults. When any other service binds to any of these port, some IPA service won't work. Regardless if it is mod_ssl or custom user service. People would probably not read FreeIPA documentation before installing mod_ssl anyway... Right. But still, we're removing the Conflicts with a package that will break IPA when installed (even indirectly). We need to be careful here. The server should now listen on both 443 with mod_nss and 10443 with mod_ssl. CLI and Web UI should continue to work, as well as cert operations like cert-show 1 - cert operations would not work if new mod_nss is not updated. That is the Apache server, right? IPA is only on 443. Yup. This just refers to testing hints above, where I suggested to configure mod_ssl to listen on some custom port to prove that both mod_ssl and mod_nss can run on the same server. Martin freeipa-mkosek-433-make-set_directive-and-get_directive-more-strict.patch ACK freeipa-mkosek-434-remove-mod_ssl-conflict.patch Just a comment on logging: [...] +print WARNING: Apache is already configured with a listener on port 443: +print line +return True Please also log these messages, otherwise the log ends up not being very helpful. Since the installation aborts, I think these should be ERROR or CRITICAL, not WARNING. Right. I used service.print_msg as you suggested on IRC. ACK, pushed to: master: 4bed0de60d5bac005c9c54c7376b8dd873d1dd1d (fixed up spec changelog) ipa-3-3: 6d24870c870d0cff0857dd7219d5475854bf8b85 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 433-434 Remove mod_ssl conflict
On 10/25/2013 03:46 PM, Petr Viktorin wrote: On 10/25/2013 02:09 PM, Martin Kosek wrote: On 10/25/2013 12:33 PM, Petr Viktorin wrote: On 10/25/2013 10:31 AM, Martin Kosek wrote: Since mod_nss-1.0.8-24, mod_nss and mod_ssl can co-exist on one machine (of course, when listening to different ports). To make sure that mod_ssl is not configured to listen on 443 (default mod_ssl configuration), add a check to the installer checking of either mod_nss or mod_ssl was configured to listen on that port. https://fedorahosted.org/freeipa/ticket/3974 [...] Right. I used service.print_msg as you suggested on IRC. ACK, pushed to: master: 4bed0de60d5bac005c9c54c7376b8dd873d1dd1d (fixed up spec changelog) ipa-3-3: 6d24870c870d0cff0857dd7219d5475854bf8b85 Jenkins found one more issue: wrong date(s) in the changelog(s). Pushed as one-liner to master: 88154b5709a898b94aa0338f16af67b37c9a95ff and two-liner to ipa-3-3: c8a4f041ced515cf164003534a07aa675d0f323a -- Petr³ From 2f09fa9b0bafb8575d8726347c00b3ea484265a3 Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Fri, 25 Oct 2013 15:30:59 +0200 Subject: [PATCH] Fix date in last changelog entry --- freeipa.spec.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/freeipa.spec.in b/freeipa.spec.in index 21ed8f90c384da56ee6fd08156e19d1beadc9c57..11ae934d928370eb13f45162a13f40a9acd64b74 100644 --- a/freeipa.spec.in +++ b/freeipa.spec.in @@ -832,7 +832,7 @@ fi %endif # ONLY_CLIENT %changelog -* Fri Aug 25 2013 Martin Kosek mko...@redhat.com - 3.3.90-4 +* Fri Oct 25 2013 Martin Kosek mko...@redhat.com - 3.3.90-4 - Remove mod_ssl conflict, it can now live with mod_nss installed * Wed Sep 4 2013 Ana Krivokapic akriv...@redhat.com - 3.3.90-3 -- 1.8.3.1 From 284e466bc3c43ef36c59cfa539b91350a9e73199 Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Fri, 25 Oct 2013 15:30:59 +0200 Subject: [PATCH] freeipa.spec: Fix changelog dates --- freeipa.spec.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/freeipa.spec.in b/freeipa.spec.in index eee32a5a2b097339f6ca432c649d4e13c54594c7..a091164907735d659be61fe29221cbce6934c77d 100644 --- a/freeipa.spec.in +++ b/freeipa.spec.in @@ -832,13 +832,13 @@ fi %endif # ONLY_CLIENT %changelog -* Fri Aug 25 2013 Martin Kosek mko...@redhat.com - 3.3.2-1 +* Fri Oct 25 2013 Martin Kosek mko...@redhat.com - 3.3.2-1 - Remove mod_ssl conflict, it can now live with mod_nss installed * Wed Sep 4 2013 Ana Krivokapic akriv...@redhat.com - 3.3.0-3 - Conform to tmpfiles.d packaging guidelines -* Wed Aug 29 2013 Petr Viktorin pvikt...@redhat.com - 3.3.0-2 +* Wed Aug 28 2013 Petr Viktorin pvikt...@redhat.com - 3.3.0-2 - Add man pages to the tests subpackage * Mon Aug 12 2013 Petr Viktorin pvikt...@redhat.com - 3.3.0-1 -- 1.8.3.1 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0076 Add test for external CA installation
On 10/22/2013 08:15 PM, Ana Krivokapic wrote: Hello, This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3819 ACK, thanks! Do we want to push this to 3.3 as well? It's a stand-alone test module; unless it's called it's as if it wasn't there. (Assuming no one runs *all* integration tests at once; I don't think anyone does) -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0121] ipatests: Add support for hosts referenced by a keyword
On 10/24/2013 12:20 PM, Tomas Babej wrote:
On 10/22/2013 10:44 AM, Petr Viktorin wrote:
On 10/22/2013 10:09 AM, Tomas Babej wrote:
On 10/22/2013 09:54 AM, Petr Viktorin wrote:
On 10/22/2013 09:20 AM, Tomas Babej wrote:
Hi,
Adds support for host definition by a environment variables of the
following form:
KEYWORDHOST__envX, where X is the number of the environment
for which host referenced by a keyword should be defined.
You can also optionally use KEYWORDHOST__IP_envX to define
the IP address directly, otherwise the framework will try to resolve
the hostname.
Adds a required_keyword_hosts attribute to the IntegrationTest class,
which can test developer use to specify the keyword hosts that this
particular test requires. If not all required keyword hosts are
available, the test will be skipped.
All keyword hosts are accessible to the IntegrationTests via the
keyword_hosts attribute, which contains a dictionary keyed by the
keywords.
Why is this necessary?
What's wrong with just extending the current scheme with more roles?
The need for keyword hosts arised with the implementation of the legacy
client test suite.
As each of these tests needs a precise type (pre-defined and
pre-configured) of VM, and not a generic client, you need to restrict
the test case to specific host type.
One test might be restricted to RHEL 5.10 with nss-pam-ldapd, another to
FreeBSD, next one to CentOS with nss-pam-ldapd, next to CentOS with old
version of SSSD...
Each testcase that needs a new type of preconfigured host, we would need
to introduce a new role, which would need to be integrated in the
framework. In such implementation, we would lose loose coupling between
the test framework and the test themselves, and make them less
pluggable.
The framework itself (excluding the configuration part) should be able
to handle this nicely using the existing role mechanism. It's true
that in some places it's currently hard-wired to just a few roles, but
supporting completely custom roles shouldn't be a problem.
I'd prefer if this system was used, rather than basically adding a
second role system, which we'd have to support even if we get rid of
the current config part.
The envvar-based configuration is not as flexible, but I'd rather make
this part somewhat unpleasant than making the framework complex. We
plan to move to a simpler configuration method anyway.
That said, you can basically keep the variable name scheme you use in
this patch; just maybe use TESTHOST rather than KEYWORDHOST.
I rewired the patch to use the current role system.
Please look if you have any additional issues.
I only have two naming nitpicks.
- The roles aren't really dynamic; eventually all the dynamicness
should be specific just to the envvar configuration system, and a few
shortcuts like `replicas` for `host_by_role('replica')`. I think extra
would be a better term.
- The environment variable names could be more descriptive. They store a
hostname, not a role, so instead of $ROLE_keyword_envX I'd prefer
$TESTHOST_role_envX
Other than that the changes look great!
--
Petr³
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0080-0081 Add userClass attributes for users and hosts
On 10/29/2013 10:49 AM, Ana Krivokapic wrote: Hello, Patch 0080 adds userClass attribute for users to IPA CLI. Patch 0081 adds userClass attribute for users and hosts to the web UI. Design page: http://www.freeipa.org/page/V3/Integration_with_a_provisioning_systems Tickets: https://fedorahosted.org/freeipa/ticket/3588 https://fedorahosted.org/freeipa/ticket/3590 And another comment: please also add any schema changes to the .ldif files. For content we're moving to use update files only, but for schema we're moving the other way, to ldif only. See https://fedorahosted.org/freeipa/ticket/3454 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 197 Track DS certificate with certmonger on replicas
On 10/17/2013 03:27 PM, Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/3975. Honza Thanks! Works for me, ACK, pushed to master: e98abdca9b4cf772e93176b42e17ec5fb5736ea4 ipa-3-3: 074816faf36650dbfa5aa8a22a3896a31b64dbf1 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 106-113 Access raw LDAP values directly from LDAPEntry
On 10/29/2013 01:34 PM, Jan Cholasta wrote: On 16.10.2013 18:13, Petr Viktorin wrote: On 10/14/2013 10:59 AM, Jan Cholasta wrote: On 10.10.2013 09:45, Jan Cholasta wrote: On 9.10.2013 13:57, Petr Viktorin wrote: [...] 109. Decode and encode attribute values in LDAPEntry on demand. The syncing looks rather over-engineered to me. Did you consider a custom MutableSequence for the values? I think it would be much cleaner in the end than merging two sets of changes together. I'm not entirely happy about it either, but it works. I did consider a custom sequence type, but I didn't feel like it was the right time to this kind of change in this patchset. Unlike the (DN, dict) - LDAPEntry transition, this change won't be backward compatible and there is a lot of isinstance(value, list) and entry[attr] = list() kind of things in the framework code. That's what I was afraid of. We could live with `isinstance(value, list)`; hopefully we could get rid of `type(value) == list` that is the real problem. With `entry[attr] = list()` we could convert automatically. But OK, let's settle for a worse solution in the meantime. To be frank I don't particularly like the LDAPEntryView. While the dict-like interface is great, there isn't a case for storing a Raw view long-term, i.e. you'd always want to do something like values = entry.raw[x] ... entry.raw[x] = new_values rather than raw = entry.raw values = raw[x] ... raw[x] = new_values The latter is confusing because LDAPEntryView and RawLDAPEntryView are two classes that have exactly the same interface, but do something different. In a duck-typed world that's a recipe for disaster. I think it would be better if the view implemented just the dict protocol, and not `conn`, `dn`, `nice`, `raw`, etc. The code would also be much simpler without the elaborate view class hierarchy. If you don't agree then at least don't make `raw` available on raw views and `nice` on nice views; the programmer *always* needs to know which version they're working with so these aren't necessary. I agree. Most of the attributes are leftovers from a previous implementation, which didn't work very well. I should have removed them a long time ago. Thanks for pointing this out! Updated the views to provide only the dict interface, removed nice and multi_value properties and also removed single_value from the raw view. Looks much better now. Hopefully _sync_attr can dissappear one day. I think it would also help (in the future?) to make the value lists more set-like, since the order doesn't matter. +1 Honza Updated patches attached. 110. It can't hurt to have this in for now. 111 - 121 look great! 106 - 121: ACK 169. For reasons I said before I'd prefer if single_value stayed a simple function. IMO a view better matches its semantics, plus I changed the code, so I would like to keep it a view, if you don't mind. OK, ACK to that one as well, but I'd rather wait a few weeks (until 3.3 churn dies out) before pushing it, since it could complicate backporting patches. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0124] ipatests: Extend clear_sssd_cache to support non-systemd
On 10/31/2013 12:38 PM, Ana Krivokapic wrote: On 10/30/2013 04:40 PM, Tomas Babej wrote: Hi, This allows us to clean sssd cache on older, non-systemd platforms. Part of: https://fedorahosted.org/freeipa/ticket/3833 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Looks good, ACK. Pushed to master: 775f2de4ecc047428034ed68dbbae934fa38de8a ipa-3-3: a4ea378e5123c9883814952df06dfe6482da307d -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0126] ipatests: Restore SELinux context after restoring files from
On 10/31/2013 01:02 PM, Ana Krivokapic wrote: On 10/30/2013 04:19 PM, Tomas Babej wrote: Hi, Without this patch, restored directories get home_t SELinux context. Part of: https://fedorahosted.org/freeipa/ticket/3833 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ACK Pushed to: master: 4fd88140b181b5f69c9312070840a4020593eb90 ipa-3-3: 63451c0b16ea501fafc2678873e604f55ae81437 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0123] ipatests: Do not use /usr/bin hardcoded paths
On 10/31/2013 02:05 PM, Ana Krivokapic wrote: On 10/30/2013 04:01 PM, Tomas Babej wrote: Hi, The RHEL 5.9 clients do not have /usr/bin symlinks. Part of: https://fedorahosted.org/freeipa/ticket/3833 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ACK Pushed to: master: 44998feace93a01b3dfda8fce6ff7aa35fffbabf ipa-3-3: d90862aaabfe663c1696152e982460821b0cb564 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0121] ipatests: Add support for hosts referenced by a keyword
On 10/30/2013 03:57 PM, Tomas Babej wrote:
On 10/29/2013 01:00 PM, Petr Viktorin wrote:
On 10/24/2013 12:20 PM, Tomas Babej wrote:
On 10/22/2013 10:44 AM, Petr Viktorin wrote:
On 10/22/2013 10:09 AM, Tomas Babej wrote:
On 10/22/2013 09:54 AM, Petr Viktorin wrote:
On 10/22/2013 09:20 AM, Tomas Babej wrote:
Hi,
Adds support for host definition by a environment variables of the
following form:
KEYWORDHOST__envX, where X is the number of the environment
for which host referenced by a keyword should be defined.
You can also optionally use KEYWORDHOST__IP_envX to define
the IP address directly, otherwise the framework will try to resolve
the hostname.
Adds a required_keyword_hosts attribute to the IntegrationTest
class,
which can test developer use to specify the keyword hosts that this
particular test requires. If not all required keyword hosts are
available, the test will be skipped.
All keyword hosts are accessible to the IntegrationTests via the
keyword_hosts attribute, which contains a dictionary keyed by the
keywords.
Why is this necessary?
What's wrong with just extending the current scheme with more roles?
The need for keyword hosts arised with the implementation of the
legacy
client test suite.
As each of these tests needs a precise type (pre-defined and
pre-configured) of VM, and not a generic client, you need to restrict
the test case to specific host type.
One test might be restricted to RHEL 5.10 with nss-pam-ldapd,
another to
FreeBSD, next one to CentOS with nss-pam-ldapd, next to CentOS with
old
version of SSSD...
Each testcase that needs a new type of preconfigured host, we would
need
to introduce a new role, which would need to be integrated in the
framework. In such implementation, we would lose loose coupling
between
the test framework and the test themselves, and make them less
pluggable.
The framework itself (excluding the configuration part) should be able
to handle this nicely using the existing role mechanism. It's true
that in some places it's currently hard-wired to just a few roles, but
supporting completely custom roles shouldn't be a problem.
I'd prefer if this system was used, rather than basically adding a
second role system, which we'd have to support even if we get rid of
the current config part.
The envvar-based configuration is not as flexible, but I'd rather make
this part somewhat unpleasant than making the framework complex. We
plan to move to a simpler configuration method anyway.
That said, you can basically keep the variable name scheme you use in
this patch; just maybe use TESTHOST rather than KEYWORDHOST.
I rewired the patch to use the current role system.
Please look if you have any additional issues.
I only have two naming nitpicks.
- The roles aren't really dynamic; eventually all the dynamicness
should be specific just to the envvar configuration system, and a few
shortcuts like `replicas` for `host_by_role('replica')`. I think
extra would be a better term.
- The environment variable names could be more descriptive. They store
a hostname, not a role, so instead of $ROLE_keyword_envX I'd prefer
$TESTHOST_role_envX
Other than that the changes look great!
Thanks, updated patch attached.
ACK, pushed to
master: b1bffb5ecad0fdaa2f560efd2b75c76bedc4423c
ipa-3-3: 960c6bf301fe3a00205a895acabe47bac5ac9349
--
Petr³
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0128] ipatests: Add integration tests for legacy clients
On 11/01/2013 03:20 PM, Tomas Babej wrote: On 11/01/2013 12:19 PM, Tomas Babej wrote: Hi, This implements the test cases for legacy clients using SSSD, nss-ldap and nss-pam-ldapd. Part of: https://fedorahosted.org/freeipa/ticket/3833 A nitpick: assert result.returncode == 0 run_command will do this for you (unless you give raiseonerr=False) -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0125] ipatests: Add which package to legacy client advice
On 11/01/2013 03:34 PM, Ana Krivokapic wrote: On 11/01/2013 03:30 PM, Tomas Babej wrote: On 11/01/2013 03:27 PM, Ana Krivokapic wrote: On 11/01/2013 03:18 PM, Tomas Babej wrote: On 10/31/2013 12:10 PM, Ana Krivokapic wrote: On 10/30/2013 04:18 PM, Tomas Babej wrote: Hi, Adds which package to the requirements, since older distros do not have it by default. Part of: https://fedorahosted.org/freeipa/ticket/3833 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel You can use the bash built-in command `command`, instead of `which`, to find out if a program exists: command -v cacertdir_rehash In other words, just replace `which` with `command -v`; there's no need to install any additional packages. -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Thanks! Updated patch attached. -- Tomas Babej Associate Software Engeneer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org Looks good! Please just amend the commit message to reflect the new content of the patch. -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc. Good catch. Fixed. -- Tomas Babej Associate Software Engeneer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org ACK Pushed to: master: 00c0878b90f0fbbe33f90cad145fefffd4aa ipa-3-3: 33ea1496572aa2f8545b853cc2b3bb4e3d5cc967 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 0315 Fix debug output in integration test
Recent ipaldap refactoring broke the simple_replication test; here is a fix. Pushed as one-liner to master: 1f6880c59059496f5002111cd0b5f16cc51961db -- Petr³ From 95c229b617342f9fb46373428abbc5ba4c7778e4 Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Fri, 1 Nov 2013 15:17:46 +0100 Subject: [PATCH] Fix debug output in integration test Recent ipaldap work has made LDAPEntry incompatible with python-ldap's LDIFWriter. Convert entry to dict before printing debug output. --- ipatests/test_integration/tasks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py index 6c36d3451cf9445450354a367618017477deefde..d30258761df6bfbf66a9b7e1a2c15114fca4c2a0 100644 --- a/ipatests/test_integration/tasks.py +++ b/ipatests/test_integration/tasks.py @@ -596,7 +596,7 @@ def _entries_to_ldif(entries): io = StringIO.StringIO() writer = LDIFWriter(io) for entry in entries: -writer.unparse(str(entry.dn), entry) +writer.unparse(str(entry.dn), dict(entry)) return io.getvalue() -- 1.8.3.1 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [RFE] Anonymous and All permissions
Hello, During discussions about fine-grained read ACIs [0], it became clear that we need to grant permissions to all authenticated and all, even anonymous users. Here is a design document for the feature: http://www.freeipa.org/page/V3/Anonymous_and_All_permissions [0] http://www.redhat.com/archives/freeipa-devel/2013-October/msg00050.html -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [RFE] Anonymous and All permissions
On 11/04/2013 04:33 PM, Martin Kosek wrote: On 11/04/2013 02:49 PM, Petr Viktorin wrote: Hello, During discussions about fine-grained read ACIs [0], it became clear that we need to grant permissions to all authenticated and all, even anonymous users. Here is a design document for the feature: http://www.freeipa.org/page/V3/Anonymous_and_All_permissions [0] http://www.redhat.com/archives/freeipa-devel/2013-October/msg00050.html Looks good to me. Pretty much reflects what were talking about in person. Kudos for also writing the Test Cases. I am just thinking we may also want to do some functional tests and e.g. add an anonymous permission to read some hidden attribute and then to try to read it with anonymous LDAP search. I'll have some functional tests in the upcoming read permissions design. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 106-113 Access raw LDAP values directly from LDAPEntry
On 10/29/2013 04:17 PM, Petr Viktorin wrote: On 10/29/2013 01:34 PM, Jan Cholasta wrote: On 16.10.2013 18:13, Petr Viktorin wrote: On 10/14/2013 10:59 AM, Jan Cholasta wrote: On 10.10.2013 09:45, Jan Cholasta wrote: On 9.10.2013 13:57, Petr Viktorin wrote: [...] 109. Decode and encode attribute values in LDAPEntry on demand. The syncing looks rather over-engineered to me. Did you consider a custom MutableSequence for the values? I think it would be much cleaner in the end than merging two sets of changes together. I'm not entirely happy about it either, but it works. I did consider a custom sequence type, but I didn't feel like it was the right time to this kind of change in this patchset. Unlike the (DN, dict) - LDAPEntry transition, this change won't be backward compatible and there is a lot of isinstance(value, list) and entry[attr] = list() kind of things in the framework code. That's what I was afraid of. We could live with `isinstance(value, list)`; hopefully we could get rid of `type(value) == list` that is the real problem. With `entry[attr] = list()` we could convert automatically. But OK, let's settle for a worse solution in the meantime. To be frank I don't particularly like the LDAPEntryView. While the dict-like interface is great, there isn't a case for storing a Raw view long-term, i.e. you'd always want to do something like values = entry.raw[x] ... entry.raw[x] = new_values rather than raw = entry.raw values = raw[x] ... raw[x] = new_values The latter is confusing because LDAPEntryView and RawLDAPEntryView are two classes that have exactly the same interface, but do something different. In a duck-typed world that's a recipe for disaster. I think it would be better if the view implemented just the dict protocol, and not `conn`, `dn`, `nice`, `raw`, etc. The code would also be much simpler without the elaborate view class hierarchy. If you don't agree then at least don't make `raw` available on raw views and `nice` on nice views; the programmer *always* needs to know which version they're working with so these aren't necessary. I agree. Most of the attributes are leftovers from a previous implementation, which didn't work very well. I should have removed them a long time ago. Thanks for pointing this out! Updated the views to provide only the dict interface, removed nice and multi_value properties and also removed single_value from the raw view. Looks much better now. Hopefully _sync_attr can dissappear one day. I think it would also help (in the future?) to make the value lists more set-like, since the order doesn't matter. +1 Honza Updated patches attached. 110. It can't hurt to have this in for now. 111 - 121 look great! 106 - 121: ACK 169. For reasons I said before I'd prefer if single_value stayed a simple function. IMO a view better matches its semantics, plus I changed the code, so I would like to keep it a view, if you don't mind. OK, ACK to that one as well, but I'd rather wait a few weeks (until 3.3 churn dies out) before pushing it, since it could complicate backporting patches. Pushed 169 to master: df5f4ee81d1aff1122dd92ab1b56eb335294c3a7 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] Summary of ipaldap changes in master
Hello, In master (IPA 3.4), an ipaldap entry's `single_value` is now a dict-like object, rather than a function: entry = ldap.get_entry(dn) print 'Hello, %s!' % entry.single_value['cn'] entry.single_value['wasGreeted'] = True Additionally, there is now a `raw` dict-like view that bypasses IPA's type conversions. This should be useful if you are working with unknown schema (e.g. AD). entry.raw['someBooleanValue'] = ['TRUE'] Happy hacking! -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Internationalized domain names in freeIPA
On 11/05/2013 05:53 PM, John Dennis wrote: On 11/05/2013 11:13 AM, Martin Basti wrote: Hi list, I'm working on ticket: https://fedorahosted.org/freeipa/ticket/3169 UTF-8 DNS names will be converted to punycode ASCII string and stored But there is a question, how to show DNS names to user (in UI or dnsrecord-show/find): * show them in punycode * convert them to UTF-8 and show * both ways * add options to show them in UTF-8 I'll be thankful for your opinion. We have a rule that all strings use UCS and that UCS be interchanged by encoding UCS text in UTF-8. Therefore it seems to me the only time punycode should ever exist is when it's necessary to encode/decode punycode for dns operations. Since punycode is a standard Python codec this should be trivial, you just need to determine where you do the encode/decode (perhaps also validating user input can be successfully encoded). In LDAP the values need to be in punycode, so bind-dyndb-ldap can process them. IMO all layers above that -- API, CLI, WebUI -- should use Unicode, except with the `--raw` flag. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [RFE] Permissions V2
Hello, I'm splitting up ACI work into several designs to make it more manageable. This one is about - Moving ACIs out of $SUFFIX - Storing all ACI data in the permission entry - Permission flag system for ensuring backwards compatibility Summary of the backcompat story: - Attributes, rights, etc. of new permissions may not be modified or read on old servers (not possible since the ACIs aren't in $SUFFIX) - Old permissions convert to new ones when they're modified on a new server - Any server can assign (or remove) both old and new permissions to privileges There is a bit of shuffling in API/CLI option names, since the API option name needs to match the LDAP attributeTypes. The WIP design document is here: http://www.freeipa.org/page/V3/Permissions_V2 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Internationalized domain names in freeIPA
On 11/05/2013 06:08 PM, John Dennis wrote: On 11/05/2013 12:04 PM, Petr Viktorin wrote: On 11/05/2013 05:53 PM, John Dennis wrote: On 11/05/2013 11:13 AM, Martin Basti wrote: Hi list, I'm working on ticket: https://fedorahosted.org/freeipa/ticket/3169 UTF-8 DNS names will be converted to punycode ASCII string and stored But there is a question, how to show DNS names to user (in UI or dnsrecord-show/find): * show them in punycode * convert them to UTF-8 and show * both ways * add options to show them in UTF-8 I'll be thankful for your opinion. We have a rule that all strings use UCS and that UCS be interchanged by encoding UCS text in UTF-8. Therefore it seems to me the only time punycode should ever exist is when it's necessary to encode/decode punycode for dns operations. Since punycode is a standard Python codec this should be trivial, you just need to determine where you do the encode/decode (perhaps also validating user input can be successfully encoded). In LDAP the values need to be in punycode, so bind-dyndb-ldap can process them. This suggests the LDAP type conversion is the right location for encode/decode. IMO all layers above that -- API, CLI, WebUI -- should use Unicode, except with the `--raw` flag. The reason for this is that UTF-8 isn't as canonical a represenation of Punicode as, say, a DN object for DNs or a bool for boolean values. Admins might reasonably want to see the raw value. Also, these values end up in DNs; I fear converting them at the LDAP wrapper level could open a can of worms. Do we have resources to give this the testing it needs? I think converting them in the DNS plugin is the way to go. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Internationalized domain names in freeIPA
On 11/07/2013 02:14 PM, Martin Kosek wrote: On 11/07/2013 01:59 PM, Petr Viktorin wrote: On 11/05/2013 06:08 PM, John Dennis wrote: On 11/05/2013 12:04 PM, Petr Viktorin wrote: On 11/05/2013 05:53 PM, John Dennis wrote: On 11/05/2013 11:13 AM, Martin Basti wrote: Hi list, I'm working on ticket: https://fedorahosted.org/freeipa/ticket/3169 UTF-8 DNS names will be converted to punycode ASCII string and stored But there is a question, how to show DNS names to user (in UI or dnsrecord-show/find): * show them in punycode * convert them to UTF-8 and show * both ways * add options to show them in UTF-8 I'll be thankful for your opinion. We have a rule that all strings use UCS and that UCS be interchanged by encoding UCS text in UTF-8. Therefore it seems to me the only time punycode should ever exist is when it's necessary to encode/decode punycode for dns operations. Since punycode is a standard Python codec this should be trivial, you just need to determine where you do the encode/decode (perhaps also validating user input can be successfully encoded). In LDAP the values need to be in punycode, so bind-dyndb-ldap can process them. This suggests the LDAP type conversion is the right location for encode/decode. IMO all layers above that -- API, CLI, WebUI -- should use Unicode, except with the `--raw` flag. The reason for this is that UTF-8 isn't as canonical a represenation of Punicode as, say, a DN object for DNs or a bool for boolean values. Admins might reasonably want to see the raw value. Also, these values end up in DNs; I fear converting them at the LDAP wrapper level could open a can of worms. Do we have resources to give this the testing it needs? I think converting them in the DNS plugin is the way to go. Just to clarify the terms here: DNS plugin === dns.py plugin in FreeIPA, not bind-dyndb-ldap. dns.py; sorry for the confusion. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0015] Add support for managing user auth types
On 11/07/2013 07:48 PM, Nathaniel McCallum wrote: On Mon, 2013-10-07 at 16:22 +0200, Petr Viktorin wrote: Sorry for the delay. On 09/25/2013 10:51 PM, Nathaniel McCallum wrote: On Mon, 2013-09-23 at 15:19 +0200, Petr Viktorin wrote: Great, we're getting close! [...] There's another test failure when trying to rename a manager user. I didn't investigate in detail why that happens. Does the failure happen without the patch? No. It seems the added objectclasses attribute conflicts with renaming a user who's a manager. Is this just a standard make check? It's the standard make test; specifically: ./make-test ipatests/test_xmlrpc/test_user_plugin.py It should pass on a newly installed server, with `make` being run in advance. Make sure to have ~/.ipa/default.conf set up. Fixed. Nathaniel Thanks! ACK, pushed to master: 3f85f09a83f1cd25078c7c11a68d457bb198d66f I've also pushed my tests from earlier in the thread: 6c7a59a906ca46d1fbdf38739ac8b33f3136de9e -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0316 Remove unused utf8_encode_value functions
On 11/06/2013 02:20 PM, Ana Krivokapic wrote: On 11/05/2013 02:02 PM, Petr Viktorin wrote: Honza's recent LDAP refactoring left some unused helper functions around. This patch removes them. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ACK Thanks! Pushed to master: 196379d126f4c86cb0979d3bae16919858bd7c19 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 198-202 Refactor indirect membership processing
On 10/31/2013 02:45 PM, Jan Cholasta wrote: Hi, the attached patches fix https://fedorahosted.org/freeipa/ticket/3971. Tested with 25000 users. Honza Patch 198: Also update ipaldap's find_entries docstring, it no longer uses IPA defaults. While you're touching this part of code, I had some other improvements in mind -- you can consider them: In find_entries, attrs_list = [a.lower() for a in attrs_list] to make sure 'memberindirect' is case insensitive In get_memberof, construct `indirect` as a set, for Ο(1) remove(). Changing MEMBERS_ALL et.al. from numbers to descriptive strings, for easier debugging. Patch 199: Looks great Patch 200: objtype, res_list, red_id, res_ctrls = result Minor typo --^ This construction won't work as you'd expect in Python 2: try: (possibly raise interesting exception) (*) except: try: (possibly raise exception to ignore) (**) except: pass raise # (***) The problem is that the exception in (**) overwrites the current active exception caught in (*). In (***) the exception from cleanup will be raised. The solution is to store the wanted exception info, including the traceback: exc_type, exc_value, exc_traceback = sys.exc_info() and then re-raise explicitly: raise exc_type, exc_value, exc_traceback Also, please log the ignored exception from cancelling the paged search. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 198-202 Refactor indirect membership processing
I hid Send by mistake; continuing review:
On 11/08/2013 03:14 PM, Petr Viktorin wrote:
On 10/31/2013 02:45 PM, Jan Cholasta wrote:
Hi,
the attached patches fix https://fedorahosted.org/freeipa/ticket/3971.
Tested with 25000 users.
Honza
Patch 198:
Also update ipaldap's find_entries docstring, it no longer uses IPA
defaults.
While you're touching this part of code, I had some other improvements
in mind -- you can consider them:
In find_entries,
attrs_list = [a.lower() for a in attrs_list]
to make sure 'memberindirect' is case insensitive
In get_memberof, construct `indirect` as a set, for Ο(1) remove().
^ ignore that, it's nuked in 201 \o/
Changing MEMBERS_ALL et.al. from numbers to descriptive strings, for
easier debugging.
^ these can be removed entirely in 201
Patch 199: Looks great
Patch 200:
objtype, res_list, red_id, res_ctrls = result
Minor typo --^
This construction won't work as you'd expect in Python 2:
try:
(possibly raise interesting exception) (*)
except:
try:
(possibly raise exception to ignore) (**)
except:
pass
raise # (***)
The problem is that the exception in (**) overwrites the current active
exception raised in (*). In (***) the exception from the cleanup will be
re-raised.
The solution is to store the wanted exception info, including the
traceback:
exc_type, exc_value, exc_traceback = sys.exc_info()
and then re-raise explicitly:
raise exc_type, exc_value, exc_traceback
Also, please log the ignored exception from cancelling the paged search.
Patch 201:
Great patch!
A nitpick, I'd rename _process_member{,of} to _process_member{,of}indirect
Patch 202: Looks good
While we're on the subject: Each Plugin has an api attribute. It would
be nice if we started preferring `self.api` instead of the global
singleton wherever possible, even though they're currently always the same.
--
Petr³
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [RFE] Permissions V2
On 11/11/2013 03:56 PM, Rob Crittenden wrote: Petr Viktorin wrote: Hello, I'm splitting up ACI work into several designs to make it more manageable. This one is about - Moving ACIs out of $SUFFIX - Storing all ACI data in the permission entry - Permission flag system for ensuring backwards compatibility Summary of the backcompat story: - Attributes, rights, etc. of new permissions may not be modified or read on old servers (not possible since the ACIs aren't in $SUFFIX) - Old permissions convert to new ones when they're modified on a new server - Any server can assign (or remove) both old and new permissions to privileges There is a bit of shuffling in API/CLI option names, since the API option name needs to match the LDAP attributeTypes. The WIP design document is here: http://www.freeipa.org/page/V3/Permissions_V2 Data in the permission entry I think the schema needs to be described better. I'm assuming that ipaPermTarget is the equivalent of current targetgroup option? And targetfilter is the equivalent of current filter option? ipaPermTarget is the raw ACI target, i.e. the DN. targetgroup is just the name If the targetgroup option is specified, it effectively just finds the group and sets the ipaPermTarget option to its DN. And if ipaPermTarget contains a group DN, targetgroup will be populated with the cn on output (in addition to ipaPermTarget with the full DN). The next update will have examples. If you are placing the ACI into the container based on type, then you probably don't need the filter within the ACI (it is implicit). Sorry; that should have been target, not fiter. The target is a bit more explicit; it has e.g. uid=* in addition to the user container DN, so I'd like to keep both. In general I think some examples would be helpful. I'll add some. Modifying and Upgrading Permissions Under what conditions would there be an old or a new permission and no associated ACI? If a command failed unexpectedly, and also failed to clean up. For example if the DS shuts down at the right time in the middle of a permission-mod. Option/Attribute mapping Performing a search on the filter is a good idea but realize that it has its limits. It is possible to create a legal filter that makes no (or little) sense. Of course. This is just a syntax check. If type is only going to specify the location of the ACI then perhaps it shouldn't be in the mutually exclusive list. Yes. Martin just pointed out ticket 2355_ (Allow filter and subtree to be added in same permission) to me today. I'll redo the mutual exclusivity so more things are possible together. _2355 :https://fedorahosted.org/freeipa/ticket/2355 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0016] Add RADIUS proxy support to ipalib CLI
On 11/12/2013 12:17 AM, Nathaniel McCallum wrote:
On Fri, 2013-11-08 at 13:26 +0100, Petr Viktorin wrote:
On 09/25/2013 10:56 PM, Nathaniel McCallum wrote:
On Fri, 2013-09-20 at 12:38 -0400, Nathaniel McCallum wrote:
On Thu, 2013-09-12 at 16:48 -0400, Nathaniel McCallum wrote:
On Thu, 2013-09-05 at 00:06 -0400, Nathaniel McCallum wrote:
patch attached
Update for ./makeapi attached.
Version 3. This should fix all the current review issues, including the
use of the referential integrity plugin. I had to make one schema change
in order to make the referential integrity modification work. Note also
that the command name prefix is changed from radius to radiusproxy.
Version 4. This patch fixes my failure to increment the minor version
number in the VERSION file.
Nathaniel
We've since decided that we'll carry LDAP content updates only in
update files, so you can leave indices.ldif referint-conf.ldif unchanged.
Schema, on the other hand, will still be in ldif files (and soon*only*
in ldif files).
Fixed.
The patch needs a rebase.
Fixed.
Also fixed: two other bugs I found when testing the above fixes. Tests
pass.
Nathaniel
Thanks for the patch!
The design page needs an update: radius-* are renamed to radiusproxy-*,
several options marked in the doc as optional are mandatory.
The password can be retrieved with radiusproxy-show --all, because it is
not blocked by LDAP ACIs. Is that intended?
ipatokenradiusserver is not validated. See validate_searchtimelimit in
the config plugin for an example validator. You can use validate_ipaddr
and validate_hostname from ipalib.util.
ipatokenusermapattribute is also not validated. Not sure if it needs to be.
The --secret CLI option does nothing (it doesn't take a value, and the
secret is prompted for whether or not the option is given). It should be
flagged no_option. (Or file an RFE for better Password semantics)
For the user commands, --radius takes the name on input, but a DN is
output. Is that expected?
I'm attaching tests I used.
@@ -640,16 +663,29 @@ class user_mod(LDAPUpdate):
entry_attrs['userpassword'] = ipa_generate_password(user_pwdchars)
# save the password so it can be displayed in post_callback
setattr(context, 'randompassword', entry_attrs['userpassword'])
-if 'ipasshpubkey' in entry_attrs or 'ipauserauthtype' in entry_attrs:
+if 'ipasshpubkey' in entry_attrs or 'ipauserauthtype' in entry_attrs
or \
+ 'ipatokenradiusconfiglink' in entry_attrs:
if 'objectclass' in entry_attrs:
obj_classes = entry_attrs['objectclass']
else:
(_dn, _entry_attrs) = ldap.get_entry(dn, ['objectclass'])
This form is deprecated. Since you don't need _dn, just do
_entry_attrs = ldap.get_entry(dn, ['objectclass'])
obj_classes = entry_attrs['objectclass'] =
_entry_attrs['objectclass']
+
if 'ipasshpubkey' in entry_attrs and 'ipasshuser' not in
obj_classes:
obj_classes.append('ipasshuser')
+
if 'ipauserauthtype' in entry_attrs and 'ipauserauthtype' not in
obj_classes:
obj_classes.append('ipauserauthtypeclass')
That should be `and 'ipauserauthtypeclass' not in obj_classes`, right?
It must have slipped an earlier review.
+
+if 'ipatokenradiusconfiglink' in entry_attrs:
+cl = entry_attrs['ipatokenradiusconfiglink']
+if cl:
+if 'ipatokenradiusproxyuser' not in
entry_attrs['objectclass']:
+
entry_attrs['objectclass'].append('ipatokenradiusproxyuser')
Nitpick: entry_attrs['objectclass'] is stored in obj_classes, you can
use that.
--
Petr³
From da40f65c4805dff4def4628df189f4b7a9de413c Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Fri, 15 Nov 2013 12:26:54 +0100
Subject: [PATCH] Add tests for the radiusproxy plugin
---
ipatests/test_xmlrpc/test_radiusproxy_plugin.py | 384
1 file changed, 384 insertions(+)
create mode 100644 ipatests/test_xmlrpc/test_radiusproxy_plugin.py
diff --git a/ipatests/test_xmlrpc/test_radiusproxy_plugin.py b/ipatests/test_xmlrpc/test_radiusproxy_plugin.py
new file mode 100644
index ..7a15a0ff0f19a3e6c0233577b2f5f16bc262f5d6
--- /dev/null
+++ b/ipatests/test_xmlrpc/test_radiusproxy_plugin.py
@@ -0,0 +1,384 @@
+# 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
Re: [Freeipa-devel] [PATCH] 463-530 First part of RCUE adoption
On 11/15/2013 02:26 PM, Petr Vobornik wrote: Hello list, this is a first part of RCUE adoption effort. Main themes of this patch set are: - use RCUE navigation https://fedorahosted.org/freeipa/ticket/3902 - new styles for textboxes, textareas, radio/checkbox buttons and buttons- part of https://fedorahosted.org/freeipa/ticket/3904 - new internal form layout (tables replaced by divs) - layout does not have fixed size https://fedorahosted.org/freeipa/ticket/3435 - new dialog styles + removed dependency on jQuery UI dialog - icons replaced by Font Awesome glyphs Example is at: http://pvoborni.fedorapeople.org/rcue/ Some reasonings and additional info: 1. RCUE includes Bootstrap which defines o lot of styles for a lot of things. That messed up the UI and therefore I did the form changes now. 2. jQuery UI is pretty big lib and we used it only for dialog and buttons. Buttons were replaced by RCUE buttons so removal of dialog dependency was a obvious step to get rid of the whole lib. The lib is removed from main UI but is still present in separate pages - will be removed later. 3. Dojo and jQuery were upgraded to latest versions.https://fedorahosted.org/freeipa/ticket/2811 This approach was ACKed by Kyle from a design perspective with a note that we will review and fixed some styling after second phase. We should not release until then. The second phase, which I'm working on right now, will consist of: * login screen https://fedorahosted.org/freeipa/ticket/3903 * new styles for standalone pages * necessary responsive enhancement (the ultimate future goal is responsive layout) It's quite a lot of patches so I did not attach them here. You can see the code in my private repo: git://fedorapeople.org/~pvoborni/freeipa.git branch 'rcue'. Wow. Do we really need all these third-party fonts and styles in our repo? install/ui/font/FontAwesome.otf | Bin 0 - 62856 bytes install/ui/font/Makefile.am | 45 +++ install/ui/font/OpenSans-Bold-webfont.eot | Bin 0 - 21190 bytes install/ui/font/OpenSans-Bold-webfont.svg | 146 ++ install/ui/font/OpenSans-Bold-webfont.ttf | Bin 0 - 21012 bytes install/ui/font/OpenSans-Bold-webfont.woff| Bin 0 - 14036 bytes install/ui/font/OpenSans-BoldItalic-webfont.eot | Bin 0 - 23510 bytes install/ui/font/OpenSans-BoldItalic-webfont.svg | 146 ++ install/ui/font/OpenSans-BoldItalic-webfont.ttf | Bin 0 - 23304 bytes install/ui/font/OpenSans-BoldItalic-webfont.woff | Bin 0 - 15572 bytes install/ui/font/OpenSans-ExtraBold-webfont.eot| Bin 0 - 21186 bytes install/ui/font/OpenSans-ExtraBold-webfont.svg| 146 ++ install/ui/font/OpenSans-ExtraBold-webfont.ttf| Bin 0 - 20988 bytes install/ui/font/OpenSans-ExtraBold-webfont.woff | Bin 0 - 14200 bytes install/ui/font/OpenSans-ExtraBoldItalic-webfont.eot | Bin 0 - 23086 bytes install/ui/font/OpenSans-ExtraBoldItalic-webfont.svg | 146 ++ install/ui/font/OpenSans-ExtraBoldItalic-webfont.ttf | Bin 0 - 22860 bytes install/ui/font/OpenSans-ExtraBoldItalic-webfont.woff | Bin 0 - 15468 bytes install/ui/font/OpenSans-Italic-webfont.eot | Bin 0 - 23866 bytes install/ui/font/OpenSans-Italic-webfont.svg | 146 ++ install/ui/font/OpenSans-Italic-webfont.ttf | Bin 0 - 23680 bytes install/ui/font/OpenSans-Italic-webfont.woff | Bin 0 - 15836 bytes install/ui/font/OpenSans-Light-webfont.eot| Bin 0 - 20886 bytes install/ui/font/OpenSans-Light-webfont.svg| 146 ++ install/ui/font/OpenSans-Light-webfont.ttf| Bin 0 - 20704 bytes install/ui/font/OpenSans-Light-webfont.woff | Bin 0 - 13972 bytes install/ui/font/OpenSans-LightItalic-webfont.eot | Bin 0 - 24074 bytes install/ui/font/OpenSans-LightItalic-webfont.svg | 146 ++ install/ui/font/OpenSans-LightItalic-webfont.ttf | Bin 0 - 23864 bytes install/ui/font/OpenSans-LightItalic-webfont.woff | Bin 0 - 15944 bytes install/ui/font/OpenSans-Regular-webfont.eot | Bin 0 - 20878 bytes install/ui/font/OpenSans-Regular-webfont.svg | 146 ++ install/ui/font/OpenSans-Regular-webfont.ttf | Bin 0 - 20688 bytes install/ui/font/OpenSans-Regular-webfont.woff | Bin 0 - 13988 bytes install/ui/font/OpenSans-Semibold-webfont.eot | Bin 0 - 21046 bytes install/ui/font/OpenSans-Semibold-webfont.svg | 146 ++ install/ui/font/OpenSans-Semibold-webfont.ttf | Bin 0 - 20852 bytes install/ui/font/OpenSans-Semibold-webfont.woff| Bin 0 - 14052 bytes
Re: [Freeipa-devel] [PATCHES] 0258-0265 Add schema updater based on IPA schema files
On 11/15/2013 02:09 PM, Petr Viktorin wrote: On 11/11/2013 04:18 PM, Ana Krivokapic wrote: On 11/11/2013 02:53 PM, Ana Krivokapic wrote: On 11/11/2013 12:32 PM, Petr Viktorin wrote: On 11/07/2013 02:34 PM, Ana Krivokapic wrote: On 11/01/2013 03:26 PM, Petr Viktorin wrote: On 09/13/2013 06:44 PM, Petr Viktorin wrote: On 08/01/2013 04:52 PM, Petr Viktorin wrote: Hello, With these patches, schema updates will be based on the ldif files we use for installation. https://fedorahosted.org/freeipa/ticket/3454 This is a RFE, here is the design doc: http://www.freeipa.org/page/V3/Improved_schema_updater I found and filed a bug in python-ldap[0]: it sometimes ignores parts of schema LDIFs when parsing them. Patch 0275 works around the bug. Please apply on top of 0258-0265 (they still apply cleanly). [0] https://bugzilla.redhat.com/show_bug.cgi?id=1007820 The recent ipaldap patches resulted in a small conflict. Attaching rebased patches. I have tested the patches and overall they seem to work fine. Some questions/comments are below. Patch 258: You catch `ldap.LOCAL_ERROR` in the `connect()` function, which is called from `__init__()`, so no need to catch it again in `__init__()`. I've added a comment instead of the try/catch Patch 259: ACK Patch 260: # Usually the modlist order does not matter. # However, for schema updates, we want 'attributetypes' before # 'objectclasses'. # A simple sort will ensure this. modlist.sort() Since `modlist` is a list of tuples, it is sorted by the first elements in the tuples, then by the seconds elements, etc. Which means the resulting list will be sorted by the modification type first (`MOD_ADD`, `MOD_DELETE`, etc), and by `attributetypes`/`objectclasses` second. Was this the desired effect? I've added a sort key; it should be safer now. Hmm, the key you added still retains the default sorting behavior - it will sort by the first elements of the tuples first. Since we need sorting by the second elements, I think the key here should be: key=lambda m: m[1].lower() Right, I see what you mean. I've made the change and tested it a bit. Patch 261: Man page updates look good, but several options in the man page have 3 dashes in the long form instead of 2. I have attached a mini-patch that fixes this along with a couple of typos in the man page. Feel free to squash it to your patch 261. Nice catch! Squashed. Patch 262: Whitespace warnings. I didn't see any with my settings; could you be more specific? $ git am ~/freeipa-pviktori-0262.3-Remove-schema-modifications-from-update-files.patch Applying: Remove schema modifications from update files /home/ana/freeipa/.git/rebase-apply/patch:497: new blank line at EOF. + /home/ana/freeipa/.git/rebase-apply/patch:693: new blank line at EOF. + warning: 2 lines add whitespace errors. Thanks! fixed. [...] I'm also seeing some errors when testing the patches. During ipa-server-install, restarting of DS is failing: [22/38]: restarting directory server ipa : CRITICAL Failed to restart the directory server (Command '/bin/systemctl restart dirsrv@IDM-LAB-ENG-BRQ-REDHAT-COM.service' returned non-zero exit status 1). See the installation log for details. The dirsrv log indicates that one of the ldif files is not syntactically valid: [11/Nov/2013:16:10:21 +0100] dse_read_one_file - The entry cn=schema in file /etc/dirsrv/slapd-IDM-LAB-ENG-BRQ-REDHAT-COM/schema/60basev3.ldif (lineno: 1) is invalid, error code 21 (Invalid syntax) - object class (2.16.840.1.113730.3.8.12.7 NAME 'ipaKrb5DelegationACL' SUP groupOfPrincipals STRUCTURAL MAY ( ipaAllowToImpersonate $ ipaAllowedTarget ) EQUALITY distinguishedNameMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.12 X-ORIGIN 'IPA v3' ): Failed to parse objectclass, error(2) at ( distinguishedNameMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.12 X-ORIGIN 'IPA v3' )) [11/Nov/2013:16:10:21 +0100] dse - Please edit the file to correct the reported problems and then restart the server. Are you seeing this in your setup? No, ipa-server-install works for me. Does this happen every time? I can't find an error in the syntax there. The bug was only visible on f20 which has a new version of 389. Thanks to Nathan Kinder for spotting the mistake (matching rulesyntax on an objectClass) for me, and to 389's new schema parser for being nice and strict. I'm only attaching the changed patch. -- Petr³ From ff72c338ad62962832ec330d00b3021731f90057 Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Mon, 29 Apr 2013 15:38:51 +0200 Subject: [PATCH] Make schema files conform to new updater The new schema updater only compares textual representations of schema elements, as formatted by python-ldap. This works well, but it is too strict for the current schema files in two ways: - For attribute names in MAY and MUST, the correct letter case must be used - AttributeTypes must specify explicit EQUALITY and SYNTAX fields even
Re: [Freeipa-devel] [PATCH] 463-530 First part of RCUE adoption
On 11/15/2013 03:28 PM, Petr Vobornik wrote: On 11/15/2013 02:40 PM, Petr Viktorin wrote: On 11/15/2013 02:26 PM, Petr Vobornik wrote: Hello list, this is a first part of RCUE adoption effort. Main themes of this patch set are: - use RCUE navigation https://fedorahosted.org/freeipa/ticket/3902 - new styles for textboxes, textareas, radio/checkbox buttons and buttons- part of https://fedorahosted.org/freeipa/ticket/3904 - new internal form layout (tables replaced by divs) - layout does not have fixed size https://fedorahosted.org/freeipa/ticket/3435 - new dialog styles + removed dependency on jQuery UI dialog - icons replaced by Font Awesome glyphs Example is at: http://pvoborni.fedorapeople.org/rcue/ Some reasonings and additional info: 1. RCUE includes Bootstrap which defines o lot of styles for a lot of things. That messed up the UI and therefore I did the form changes now. 2. jQuery UI is pretty big lib and we used it only for dialog and buttons. Buttons were replaced by RCUE buttons so removal of dialog dependency was a obvious step to get rid of the whole lib. The lib is removed from main UI but is still present in separate pages - will be removed later. 3. Dojo and jQuery were upgraded to latest versions.https://fedorahosted.org/freeipa/ticket/2811 This approach was ACKed by Kyle from a design perspective with a note that we will review and fixed some styling after second phase. We should not release until then. The second phase, which I'm working on right now, will consist of: * login screen https://fedorahosted.org/freeipa/ticket/3903 * new styles for standalone pages * necessary responsive enhancement (the ultimate future goal is responsive layout) It's quite a lot of patches so I did not attach them here. You can see the code in my private repo: git://fedorapeople.org/~pvoborni/freeipa.git branch 'rcue'. Wow. Do we really need all these third-party fonts and styles in our repo? It's common in Web development to offer all versions and let the browser to choose one. Since FreeIPA Web UI supports IE9+ we can safely reduce the font files only to .woff fonts http://caniuse.com/woff. We can also discard all Italic fonts (not used atm). Fedora 20 has a new feature called Web Assets http://fedoraproject.org/wiki/Changes/Web_Assets which should solve such bundles. I'm not convinced that it's in usable state atm. That doesn't answer the question of why we need third-party compiled assets in the repo. Fedora spec files can also have multiple Sources, if we need to bundle for distribution. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0258-0265 Add schema updater based on IPA schema files
On 11/18/2013 04:12 PM, Ana Krivokapic wrote: On 11/15/2013 05:28 PM, Petr Viktorin wrote: On 11/15/2013 02:09 PM, Petr Viktorin wrote: On 11/11/2013 04:18 PM, Ana Krivokapic wrote: On 11/11/2013 02:53 PM, Ana Krivokapic wrote: On 11/11/2013 12:32 PM, Petr Viktorin wrote: On 11/07/2013 02:34 PM, Ana Krivokapic wrote: On 11/01/2013 03:26 PM, Petr Viktorin wrote: On 09/13/2013 06:44 PM, Petr Viktorin wrote: On 08/01/2013 04:52 PM, Petr Viktorin wrote: Hello, With these patches, schema updates will be based on the ldif files we use for installation. https://fedorahosted.org/freeipa/ticket/3454 This is a RFE, here is the design doc: http://www.freeipa.org/page/V3/Improved_schema_updater I found and filed a bug in python-ldap[0]: it sometimes ignores parts of schema LDIFs when parsing them. Patch 0275 works around the bug. Please apply on top of 0258-0265 (they still apply cleanly). [0] https://bugzilla.redhat.com/show_bug.cgi?id=1007820 The recent ipaldap patches resulted in a small conflict. Attaching rebased patches. I have tested the patches and overall they seem to work fine. Some questions/comments are below. Patch 258: You catch `ldap.LOCAL_ERROR` in the `connect()` function, which is called from `__init__()`, so no need to catch it again in `__init__()`. I've added a comment instead of the try/catch Patch 259: ACK Patch 260: # Usually the modlist order does not matter. # However, for schema updates, we want 'attributetypes' before # 'objectclasses'. # A simple sort will ensure this. modlist.sort() Since `modlist` is a list of tuples, it is sorted by the first elements in the tuples, then by the seconds elements, etc. Which means the resulting list will be sorted by the modification type first (`MOD_ADD`, `MOD_DELETE`, etc), and by `attributetypes`/`objectclasses` second. Was this the desired effect? I've added a sort key; it should be safer now. Hmm, the key you added still retains the default sorting behavior - it will sort by the first elements of the tuples first. Since we need sorting by the second elements, I think the key here should be: key=lambda m: m[1].lower() Right, I see what you mean. I've made the change and tested it a bit. Patch 261: Man page updates look good, but several options in the man page have 3 dashes in the long form instead of 2. I have attached a mini-patch that fixes this along with a couple of typos in the man page. Feel free to squash it to your patch 261. Nice catch! Squashed. Patch 262: Whitespace warnings. I didn't see any with my settings; could you be more specific? $ git am ~/freeipa-pviktori-0262.3-Remove-schema-modifications-from-update-files.patch Applying: Remove schema modifications from update files /home/ana/freeipa/.git/rebase-apply/patch:497: new blank line at EOF. + /home/ana/freeipa/.git/rebase-apply/patch:693: new blank line at EOF. + warning: 2 lines add whitespace errors. Thanks! fixed. [...] I'm also seeing some errors when testing the patches. During ipa-server-install, restarting of DS is failing: [22/38]: restarting directory server ipa : CRITICAL Failed to restart the directory server (Command '/bin/systemctl restart dirsrv@IDM-LAB-ENG-BRQ-REDHAT-COM.service' returned non-zero exit status 1). See the installation log for details. The dirsrv log indicates that one of the ldif files is not syntactically valid: [11/Nov/2013:16:10:21 +0100] dse_read_one_file - The entry cn=schema in file /etc/dirsrv/slapd-IDM-LAB-ENG-BRQ-REDHAT-COM/schema/60basev3.ldif (lineno: 1) is invalid, error code 21 (Invalid syntax) - object class (2.16.840.1.113730.3.8.12.7 NAME 'ipaKrb5DelegationACL' SUP groupOfPrincipals STRUCTURAL MAY ( ipaAllowToImpersonate $ ipaAllowedTarget ) EQUALITY distinguishedNameMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.12 X-ORIGIN 'IPA v3' ): Failed to parse objectclass, error(2) at ( distinguishedNameMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.12 X-ORIGIN 'IPA v3' )) [11/Nov/2013:16:10:21 +0100] dse - Please edit the file to correct the reported problems and then restart the server. Are you seeing this in your setup? No, ipa-server-install works for me. Does this happen every time? I can't find an error in the syntax there. The bug was only visible on f20 which has a new version of 389. Thanks to Nathan Kinder for spotting the mistake (matching rulesyntax on an objectClass) for me, and to 389's new schema parser for being nice and strict. I'm only attaching the changed patch. Thanks, it works well now. ACK for the whole patch set. Thank you! Pushed to master: 2bc7803b69d15a246486ab5c8a44ead7593e8e90 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0016] Add RADIUS proxy support to ipalib CLI
On 11/15/2013 12:34 PM, Petr Viktorin wrote: On 11/12/2013 12:17 AM, Nathaniel McCallum wrote: On Fri, 2013-11-08 at 13:26 +0100, Petr Viktorin wrote: We've since decided that we'll carry LDAP content updates only in update files, so you can leave indices.ldif referint-conf.ldif unchanged. Schema, on the other hand, will still be in ldif files (and soon *only* in ldif files). Heads up: this was merged. When you get a merge conflict just remove the schema update file. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] ACI changes overview/roadmap
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
Re: [Freeipa-devel] [PATCH] 463-530 First part of RCUE adoption
On 11/18/2013 06:17 PM, Petr Vobornik wrote: On 11/15/2013 05:43 PM, Petr Viktorin wrote: On 11/15/2013 03:28 PM, Petr Vobornik wrote: On 11/15/2013 02:40 PM, Petr Viktorin wrote: On 11/15/2013 02:26 PM, Petr Vobornik wrote: [...] It's quite a lot of patches so I did not attach them here. You can see the code in my private repo: git://fedorapeople.org/~pvoborni/freeipa.git branch 'rcue'. Wow. Do we really need all these third-party fonts and styles in our repo? It's common in Web development to offer all versions and let the browser to choose one. Since FreeIPA Web UI supports IE9+ we can safely reduce the font files only to .woff fonts http://caniuse.com/woff. We can also discard all Italic fonts (not used atm). Fedora 20 has a new feature called Web Assets http://fedoraproject.org/wiki/Changes/Web_Assets which should solve such bundles. I'm not convinced that it's in usable state atm. That doesn't answer the question of why we need third-party compiled assets in the repo. Fedora spec files can also have multiple Sources, if we need to bundle for distribution. I worry that it will be just source of build errors and such. For example I would like to have access to the files during development without running rpm build or using crystal ball to figure out what is needed and where to get it. A curious requirement, bundling everything in the Git repo. I'm afraid I don't really understand your thinking here. In the world I live in, a repo should contain upstream source files. Third-party dependencies must be listed, and need to be installed for building/using the project, and compiled artifacts should be compiled from sources. Having third-party compiled stuff means bundling, forking, and other similar swear words. Needing a crystal ball to locate the source is a packager's nightmare. And this patchset put us in that situation, for any kind of checking if these are up to date or what the licenses are. Now I would suggest to use Bower http://bower.io/ to solve it, but it requires Node.js so I won't do that. Here is some information about possible sources. Does it look usable to you in any way? 1. Open Sans - hard to find usable source, they should be in googlefontdirectory http://code.google.com/p/googlefontdirectory/ but that's 2.5GB+ Mercuial repo... - some unofficial source (can be used by Bower): https://github.com/FontFaceKit/open-sans It has a bit different font-face declaration which would require additional changes 2. Overpass - official distribution [1] contains only .ttf fonts - .woff version is in git repo https://git.fedorahosted.org/cgit/overpass-fonts.git 3. Font Awesome - provides tarball http://fontawesome.io/assets/font-awesome-4.0.3.zip - should not be a problem if I don't count .less files which required some changes because lesscpy could not process them So we have a fork there as well? Did you try to submit the patch upstream? So, now we need to package these so that we can depend on them, right? At least that's how it works with any other project we want to depend on if it's not in Fedora yet. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0014, 0016 [RFE] ipa migrate-ds should have an argument to specify cert to use for DS connection
On 10/21/2013 10:29 AM, Martin Basti wrote: On Mon, 2013-10-21 at 09:29 +0200, Martin Kosek wrote: On 10/18/2013 05:00 PM, Martin Basti wrote: Patch attached. Ticket: https://fedorahosted.org/freeipa/ticket/3243 I did not test the patch, just looked at the code and I have few comments: 1) Please put the ipalib/cli.py change to a sepparate change, we may want to backport it separately some day and this will make it easier. Separated. Patch 0014-2 -- CLI Nitpick: if you switched them around it would be easier to read*: if raw: decode elif required: raise *(at least for me, I need way too much concentration to process boolean logic) Patch 0016 -- migration We have a utility, ipautil.write_tmp_file, that should do what you need here. With this the created file is removed some time later when there are no more references to the file object, so no need for the try: block. (btw, if the try block was necessary, it should have also covered the write() call.) Also, since you changed the API, please run ./makeapi and bump the API version in the VERSION file. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 439 Allow kernel keyring CCACHE when supported
On 11/05/2013 07:22 PM, Martin Kosek wrote: Server and client installer should allow kernel keyring ccache when supported. The patch needs a rebase. Can you add a function to check if persistent key is supported? It would remove some code duplication. How do I enable the kernel keyring? On f20 I get this: 2013-11-19T11:28:07Z DEBUG Starting external process 2013-11-19T11:28:07Z DEBUG args=keyctl get_persistent @s 0 2013-11-19T11:28:07Z DEBUG Process finished, return code=1 2013-11-19T11:28:07Z DEBUG stdout= 2013-11-19T11:28:07Z DEBUG stderr=keyctl_get_persistent: Key has been revoked -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0080-0081 Add userClass attributes for users and hosts
On 11/19/2013 12:58 PM, Ana Krivokapic wrote: On 11/19/2013 12:52 PM, Ana Krivokapic wrote: On 11/14/2013 10:04 AM, Petr Vobornik wrote: On 11/13/2013 01:33 PM, Ana Krivokapic wrote: On 11/12/2013 01:27 PM, Ana Krivokapic wrote: On 10/30/2013 09:56 PM, Martin Kosek wrote: - Original Message - From: Simo Sorces...@redhat.com To: Ana Krivokapicakriv...@redhat.com Cc: Martin Kosekmko...@redhat.com, freeipa-devel freeipa-devel@redhat.com Sent: Wednesday, October 30, 2013 7:11:20 PM Subject: Re: [Freeipa-devel] [PATCHES] 0080-0081 Add userClass attributes for users and hosts On Wed, 2013-10-30 at 19:01 +0100, Ana Krivokapic wrote: On 10/29/2013 02:04 PM, Simo Sorce wrote: On Tue, 2013-10-29 at 12:42 +0100, Martin Kosek wrote: On 10/29/2013 10:49 AM, Ana Krivokapic wrote: Hello, Patch 0080 adds userClass attribute for users to IPA CLI. Patch 0081 adds userClass attribute for users and hosts to the web UI. Design page: http://www.freeipa.org/page/V3/Integration_with_a_provisioning_systems Tickets: https://fedorahosted.org/freeipa/ticket/3588 https://fedorahosted.org/freeipa/ticket/3590 NACK to just extending posixAccount objectclass. This is a standard objectclass defined by RFC 2307 and we cannot just simply extend and overwrite it as we wish. Uhh indeed this is a big No-no. We will need to come up with some custom objectclass, like ipaUser. This is the reason why I wrote to ticket A second goal of this ticket is to review current objectClass hierarchy of users and do changes if needed. so that we can pick the best option where to place it. userClass is used in ipaHost, so I guess it could be instead add to an ipa objectclass. ipaObject might be used perhaps, otherwise we'll need a new ipaUser objectlass. Simo. If there are no objections to using the ipaObject objectclass, the attached patches implement this approach. After some thinking ipaObject is more generic than just users, not sure that attaching userClass there is appropriate. I think we really need ipaUser at this point. +1. I also do not think that ipaObject is the right OC to place the attribute, it is just too general. Let's go with the ipaUser objectClass, looking something like that: ( OID NAME 'ipaUser' AUXILIARY MUST ( uid ) MAY ( userClass ) X-ORIGIN 'IPA v3' ) We will need to add the OC when needed, we cannot just add it to default list. Ideally, we could also implement https://fedorahosted.org/freeipa/ticket/3922 in scope of this effort as this need to add additional OCs is piling up. Martin This implementation introduces a new objectclass 'ipaUser'. The web UI patch needed an update as well, as we need to allow writing the userClass attribute even when the ipaUser objectclass is not (yet) set on the user object. Thanks Petr for pointing it out. Attaching both patches again (the CLI patch has not changed since the last iteration). ACK for Web UI patch (81-03) 80-03 seems good to me as well, but I don't feel strong about my ability to judge correctness of ldap/ipalib part so I let somebody else to look at it. Patch 80 rebased. Patch 80 fixed to use the correct capitalization of 'objectClasses'. Thanks! Works for me, tests pass, ACK. Pushed to master: afbf528a838248f9c0a010c5be91faece1bbe743 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 463-530 First part of RCUE adoption
On 11/19/2013 01:27 PM, Petr Vobornik wrote:
On 11/18/2013 08:47 PM, Petr Viktorin wrote:
On 11/18/2013 06:17 PM, Petr Vobornik wrote:
On 11/15/2013 05:43 PM, Petr Viktorin wrote:
On 11/15/2013 03:28 PM, Petr Vobornik wrote:
On 11/15/2013 02:40 PM, Petr Viktorin wrote:
On 11/15/2013 02:26 PM, Petr Vobornik wrote:
[...]
It's quite a lot of patches so I did not attach them here. You can
see
the code in my private repo:
git://fedorapeople.org/~pvoborni/freeipa.git branch 'rcue'.
Wow. Do we really need all these third-party fonts and styles in our
repo?
It's common in Web development to offer all versions and let the
browser
to choose one.
Since FreeIPA Web UI supports IE9+ we can safely reduce the font files
only to .woff fonts http://caniuse.com/woff. We can also discard all
Italic fonts (not used atm).
Fedora 20 has a new feature called Web Assets
http://fedoraproject.org/wiki/Changes/Web_Assets which should solve
such bundles. I'm not convinced that it's in usable state atm.
That doesn't answer the question of why we need third-party compiled
assets in the repo.
Fedora spec files can also have multiple Sources, if we need to bundle
for distribution.
I worry that it will be just source of build errors and such. For
example I would like to have access to the files during development
without running rpm build or using crystal ball to figure out what is
needed and where to get it.
A curious requirement, bundling everything in the Git repo. I'm afraid I
don't really understand your thinking here.
In the world I live in, a repo should contain upstream source files.
Third-party dependencies must be listed, and need to be installed for
building/using the project, and compiled artifacts should be compiled
from sources.
Having third-party compiled stuff means bundling, forking, and other
similar swear words.
Needing a crystal ball to locate the source is a packager's nightmare.
And this patchset put us in that situation, for any kind of checking if
these are up to date or what the licenses are.
Now I would suggest to use Bower http://bower.io/ to solve it, but it
requires Node.js so I won't do that.
Here is some information about possible sources. Does it look usable to
you in any way?
1. Open Sans
- hard to find usable source, they should be in googlefontdirectory
http://code.google.com/p/googlefontdirectory/ but that's 2.5GB+
Mercuial repo...
- some unofficial source (can be used by Bower):
https://github.com/FontFaceKit/open-sans It has a bit different
font-face declaration which would require additional changes
2. Overpass
- official distribution [1] contains only .ttf fonts
- .woff version is in git repo
https://git.fedorahosted.org/cgit/overpass-fonts.git
3. Font Awesome
- provides tarball http://fontawesome.io/assets/font-awesome-4.0.3.zip
- should not be a problem if I don't count .less files which required
some changes because lesscpy could not process them
So we have a fork there as well?
Did you try to submit the patch upstream?
So, now we need to package these so that we can depend on them, right?
At least that's how it works with any other project we want to depend on
if it's not in Fedora yet.
Fedora is not ready for packaging web libraries. We can continue with
the discussion after release of Fedora 21.
https://fedoraproject.org/wiki/Changes/Web_Assets#Scope
I must be reading the page wrong. For F20 it links to approved
guidelines[0] that explicitly say to use/link system fonts from
%{_datadir}/fonts, and packaging fonts elsewhere is prohibited.
Also web-assets is in f19, so only the httpd config is missing. As I
read it, we're supposed to provide that ourselves for now.
Until then we have to bundle unless we use system like aforementioned
Bower.
[0] https://fedoraproject.org/wiki/Packaging:Web_Assets#Fonts
--
Petr³
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0130] platform: Add Fedora 19 platform file
On 11/15/2013 02:40 PM, Ana Krivokapic wrote: On 11/13/2013 02:56 PM, Tomas Babej wrote: Hi, Part of: https://fedorahosted.org/freeipa/ticket/3504 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ACK Pushed to master: 60b472479d6427243b5ef51c4dd60cdcd9e52afd -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 111] ipa-client-install: Publish CA certificate to systemwide store
On 11/20/2013 12:59 PM, Ana Krivokapic wrote: On 11/18/2013 01:54 PM, Tomas Babej wrote: [...] Updated patch attached. Looks good, ACK. Pushed to master: 4a0e91449e2b65304ae8d660d1a480200b1a13d3 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0276-0277 Break long doc strings for translations
On 11/20/2013 03:42 PM, Ana Krivokapic wrote: On 10/09/2013 04:11 PM, Petr Viktorin wrote: On 09/16/2013 05:13 PM, Petr Viktorin wrote: Hello, The first patch allow concatenating LazyText objects using `+`. This means we can break up long docstrings into multiple parts. Translators can then work on each part separately, and the whole translation is not lost when a small part is changed or added. The second patch splits up the docstring for Host*. I chose Host because it was updated since last release, so translators would have to-retranslate the text. In the future, whenever a long docstring is changed it should be broken up like this. The patch also updates translations, and adds the broken-up texts for French and Ukraininan. You can test by viewing Host help in one of these languages -- only the new section should be untranslated. Rebased to current master. I've added a patch to update translations, please apply it before the other two. It makes the functional changes a bit clearer. Test with: $ LANG=fr_FR ipa help host ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Looks good to me. ACK. Thank you! Pushed to master: 56e3e12f129fa43c4ef66dce4bee55dcd7cd38b6 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 0319-0320 test_integration: Set up DNS on replicas
Hello,
This should fix tests failing on Beaker when the test controller and
master share the same machine.
The second patch adds more debugging output to the code that fails.
https://fedorahosted.org/freeipa/ticket/4038
--
Petr³
From 492762932ecc128919fa6adfa2a2f0f895f879b9 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Thu, 21 Nov 2013 12:06:29 +0100
Subject: [PATCH] test_integration: Set up DNS on replicas
https://fedorahosted.org/freeipa/ticket/4038
---
ipatests/test_integration/tasks.py | 2 ++
1 file changed, 2 insertions(+)
diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index cee54768312c9cf0d79b9137f134af718b0a3b5f..10d7ef8869d5e36dfcdff59c1f50e3d725aaf6f7 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -203,6 +203,8 @@ def install_replica(master, replica, setup_ca=True):
'-p', replica.config.dirman_password,
'-w', replica.config.admin_password,
'--ip-address', replica.ip,
+'--setup-dns',
+'--forwarder', replica.config.dns_forwarder,
replica_filename]
if setup_ca:
args.append('--setup-ca')
--
1.8.3.1
From d25fc47487d7893e4436e2972ec50ad4d05ce8a8 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Thu, 21 Nov 2013 13:57:47 +0100
Subject: [PATCH] test_integration: Add debugging info to Host.ldap_connect
---
ipatests/test_integration/host.py | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/ipatests/test_integration/host.py b/ipatests/test_integration/host.py
index 02c82b372ce2805c0ca922319f5de1cd29b0ed82..eb65a62df14f017638a35f21dfb3610d54457c21 100644
--- a/ipatests/test_integration/host.py
+++ b/ipatests/test_integration/host.py
@@ -157,7 +157,11 @@ def put_file_contents(self, filename, contents):
def ldap_connect(self):
Return an LDAPClient authenticated to this host as directory manager
-self.log.info('Connecting to LDAP')
+# First run ping (if available) to make sure hostname is reachable
+self.run_command(['ping', '-c1', self.external_hostname],
+ raiseonerr=False)
+
+self.log.info('Connecting to LDAP at %s', self.external_hostname)
ldap = IPAdmin(self.external_hostname)
binddn = self.config.dirman_dn
self.log.info('LDAP bind as %s' % binddn)
--
1.8.3.1
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0319-0320 test_integration: Set up DNS on replicas
On 11/21/2013 02:04 PM, Petr Viktorin wrote: Hello, This should fix tests failing on Beaker when the test controller and master share the same machine. The second patch adds more debugging output to the code that fails. https://fedorahosted.org/freeipa/ticket/4038 Self-NACK for now; I'll investigate the issue a bit more. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0317 Improve LDAPEntry.__repr__ for freshly created entries
On 11/25/2013 01:05 PM, Jan Cholasta wrote: On 6.11.2013 13:28, Petr Viktorin wrote: Hello Honza, This is a simple enough patch, but I'd like you to check if it's consistent with your vision of the framework. I used self._raw here deliberately, so that calling repr() on an LDAPEntry does not change its internal state. I agree that using self._raw alone is insufficient, but I'd like to keep the no changes behavior, perhaps using something like this: data = dict(self._raw) data.update(self._nice) return '%s(%r, %r)' % (type(self).__name__, self._dn, data) That makes sense. Newly created entries have None values in _nice so I filtered them out here. -- Petr³ From 17b9b56ecb7964a5f7723c1e7c9de68e95253932 Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Wed, 6 Nov 2013 12:40:02 +0100 Subject: [PATCH] Improve LDAPEntry.__repr__ for freshly created entries Creating a LDAPEntry from dict does not set the raw entries, to display everything we need to combine the underlying data. https://fedorahosted.org/freeipa/ticket/4015 --- ipapython/ipaldap.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py index 027bfa979c0d239cb1c042e5b1ddb8f82f0eb2ad..8dbc09000b2c8db1de5d0350045bf2d9ae89934b 100644 --- a/ipapython/ipaldap.py +++ b/ipapython/ipaldap.py @@ -719,7 +719,9 @@ def orig_data(self): return self._orig def __repr__(self): -return '%s(%r, %r)' % (type(self).__name__, self._dn, self._raw) +data = dict(self._raw) +data.update((k, v) for k, v in self._nice.items() if v is not None) +return '%s(%r, %r)' % (type(self).__name__, self._dn, data) def copy(self): return LDAPEntry(self) -- 1.8.3.1 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 203 Remove mod_ssl port workaround
On 11/26/2013 12:34 PM, Alexander Bokovoy wrote: On Tue, 26 Nov 2013, Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/4021. Honza -- Jan Cholasta ACK. Pushed to: master: f20577ddc4ab40c2365c8abaa703d96019ec4eef ipa-3-3: 3a11044664341257a3929da2db1c493659515eec P.S. When do we start removing changelog entries from the spec.in in git master? :) Soon :) -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 0321 Remove changelog from the spec
The changelog was useless and caused unnecessary rebase conflicts. Let's kill it. -- Petr³ From fe9d847fa39e3683ada3b7d12f3643ae9433bf45 Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Tue, 26 Nov 2013 13:06:07 +0100 Subject: [PATCH] Remove changelog from the spec The project's history is kept in Git. We used the spec changelog for changes to the spec itself, which doesn't make much sense. Downstreams like Fedora use their own changelog anyway. A single entry is left for tools that expect a changelog. --- freeipa.spec.in | 735 +--- 1 file changed, 3 insertions(+), 732 deletions(-) diff --git a/freeipa.spec.in b/freeipa.spec.in index 2a738dda1220615531f0a3f99137638cd9257111..73c0d46e94f795eef95aa324f4d72df29b7d47ce 100644 --- a/freeipa.spec.in +++ b/freeipa.spec.in @@ -839,735 +839,6 @@ fi %endif # ONLY_CLIENT %changelog -* Tue Nov 26 2013 Jan Cholasta jchol...@redhat.com - 3.3.90-6 -- Set minimum version of httpd to 2.4.6-6 -- Set minimum version of mod_nss to 1.0.8-26 - -* Tue Nov 12 2013 Tomas Babejtba...@redhat.com - 3.3.90-5 -- Add Fedora 19 platform files - -* Fri Oct 25 2013 Martin Kosek mko...@redhat.com - 3.3.90-4 -- Remove mod_ssl conflict, it can now live with mod_nss installed - -* Wed Sep 4 2013 Ana Krivokapic akriv...@redhat.com - 3.3.90-3 -- Conform to tmpfiles.d packaging guidelines - -* Wed Aug 14 2013 Petr Viktorin pvikt...@redhat.com - 3.3.90-2 -- Add man pages to the tests subpackage - -* Mon Aug 12 2013 Petr Viktorin pvikt...@redhat.com - 3.3.90-1 -- Downgrade required version of python-paramiko for the tests subpackage - -* Thu Aug 8 2013 Martin Kosek mko...@redhat.com - 3.2.99-13 -- Require slapi-nis 0.47.7 and sssd 1.11.0-0.1.beta2 required for core - features of 3.3.0 release - -* Fri Jul 26 2013 Martin Kosek mko...@redhat.com - 3.2.99-12 -- Require pki-ca 10.0.4 which fixes external CA installation (#986901) - -* Wed Jul 24 2013 Petr Viktorin pvikt...@redhat.com - 3.2.99-11 -- Add tar and xz dependencies to freeipa-tests - -* Wed Jul 24 2013 Tomas Babej tba...@redhat.com - 3.2.99-10 -- Move requirement for keyutils from freeipa-server to freeipa-python - -* Wed Jul 24 2013 Martin Kosek mko...@redhat.com - 3.2.99-9 -- Bump minimum version of sssd to 1.10.92 to pick up latest SSSD 1.11 Beta - development - -* Thu Jul 18 2013 Ana Krivokapic akriv...@redhat.com - 3.2.99-8 -- Bump minimum version of sssd to 1.10.90 for the 'ipa_server_mode' option. - -* Wed Jul 17 2013 Martin Kosek mko...@redhat.com - 3.2.99-7 -- Require selinux-policy 3.12.1-65 containing missing policy after removal of - freeipa-server-selinux subpackage - -* Tue Jul 16 2013 Tomas Babej tba...@redhat.com - 3.2.99-6 -- Do not create /var/lib/ipa/pki-ca/publish, retain reference as ghost - -* Thu Jul 11 2013 Martin Kosek mko...@redhat.com - 3.2.99-5 -- Run ipa-upgradeconfig and server restart in posttrans to avoid inconsistency - issues when there are still old parts of software (like entitlements plugin) - -* Wed Jul 10 2013 Ana Krivokapic akriv...@redhat.com - 3.2.99-4 -- Bump minimum version of 389-ds-base to 1.3.1.3 for user password change fix. - -* Wed Jun 26 2013 Jan Cholasta jchol...@redhat.com - 3.2.99-3 -- Bump minimum version of 389-ds-base to 1.3.1.1 for SASL mapping priority - support. - -* Mon Jun 17 2013 Petr Viktorin pvikt...@redhat.com - 3.2.99-2 -- Add the freeipa-tests subpackage - -* Thu Jun 13 2013 Martin Kosek mko...@redhat.com - 3.2.99-1 -- Drop freeipa-server-selinux subpackage -- Drop redundant directory /var/cache/ipa/sessions - -* Fri May 10 2013 Martin Kosek mko...@redhat.com - 3.1.99-13 -- Add requires for openldap-2.4.35-4 to pickup fixed SASL_NOCANON behavior for - socket based connections (#960222) - -* Tue May 7 2013 Petr Viktorin pvikt...@redhat.com - 3.1.99-12 -- Require libsss_nss_idmap-python in Fedora 19+ - -* Mon May 6 2013 Petr Vobornik pvobo...@redhat.com - 3.1.99-11 -- Web UI plugins - -* Fri May 3 2013 Rob Crittenden rcrit...@redhat.com - 3.1.99-10 -- Require pki-ca 10.0.2 for 501 response code on find for d9 - d10 upgrades - -* Tue Apr 30 2013 Rob Crittenden rcrit...@redhat.com - 3.1.99-9 -- Add Conflicts on nss-pam-ldapd 0.8.4. The mapping from uniqueMember to - member is now done automatically and having it in the config file raises - an error. - -* Tue Apr 30 2013 Jan Cholasta jchol...@redhat.com - 3.1.99-8 -- Add triggerin scriptlet to update sshd_config on openssh-server update - -* Thu Apr 25 2013 Rob Crittenden rcrit...@redhat.com - 3.1.99-7 -- Update nss and nss-tools dependency to fix certutil problem (#872761) - -* Mon Apr 15 2013 Martin Kosek mko...@redhat.com - 3.1.99-6 -- Require samba 4.0.5, includes new passdb API -- Require krb5 1.11.2-1, fixes missing PAC issue -- Change permissions on backup dir to 700 - -* Fri Apr 5 2013 Rob Crittenden rcrit...@redhat.com - 3.1.99-5 -- Add backup and restore -- Own /var/lib/ipa/backup - -* Thu Apr 4 2013 Alexander Bokovoy
Re: [Freeipa-devel] [PATCH] 0317 Improve LDAPEntry.__repr__ for freshly created entries
On 11/26/2013 09:57 AM, Jan Cholasta wrote: On 25.11.2013 14:41, Petr Viktorin wrote: On 11/25/2013 01:05 PM, Jan Cholasta wrote: On 6.11.2013 13:28, Petr Viktorin wrote: Hello Honza, This is a simple enough patch, but I'd like you to check if it's consistent with your vision of the framework. I used self._raw here deliberately, so that calling repr() on an LDAPEntry does not change its internal state. I agree that using self._raw alone is insufficient, but I'd like to keep the no changes behavior, perhaps using something like this: data = dict(self._raw) data.update(self._nice) return '%s(%r, %r)' % (type(self).__name__, self._dn, data) That makes sense. Newly created entries have None values in _nice so I filtered them out here. Nitpick: use iteritems() instead of items(). Besides that, ACK. Thanks! Changed and pused to master: 76c7f24919d30fdd53e4a1cd32880b55c2437ace -- Petr³ From bcb542e17112428ae754eb1afbbfb9eedb52eec6 Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Wed, 6 Nov 2013 12:40:02 +0100 Subject: [PATCH] Improve LDAPEntry.__repr__ for freshly created entries Creating a LDAPEntry from dict does not set the raw entries, to display everything we need to combine the underlying data. https://fedorahosted.org/freeipa/ticket/4015 --- ipapython/ipaldap.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py index 027bfa979c0d239cb1c042e5b1ddb8f82f0eb2ad..41ae9ec3fbd706972bd4de79bbaea998ac90b8f5 100644 --- a/ipapython/ipaldap.py +++ b/ipapython/ipaldap.py @@ -719,7 +719,9 @@ def orig_data(self): return self._orig def __repr__(self): -return '%s(%r, %r)' % (type(self).__name__, self._dn, self._raw) +data = dict(self._raw) +data.update((k, v) for k, v in self._nice.iteritems() if v is not None) +return '%s(%r, %r)' % (type(self).__name__, self._dn, data) def copy(self): return LDAPEntry(self) -- 1.8.3.1 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0321 Remove changelog from the spec
On 11/26/2013 01:27 PM, Alexander Bokovoy wrote: On Tue, 26 Nov 2013, Petr Viktorin wrote: The changelog was useless and caused unnecessary rebase conflicts. Let's kill it. -- Petr³ From fe9d847fa39e3683ada3b7d12f3643ae9433bf45 Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Tue, 26 Nov 2013 13:06:07 +0100 Subject: [PATCH] Remove changelog from the spec The project's history is kept in Git. We used the spec changelog for changes to the spec itself, which doesn't make much sense. Downstreams like Fedora use their own changelog anyway. A single entry is left for tools that expect a changelog. ACK with one change: +* Tue Nov 26 2013 Petr Viktorinpvikt...@redhat.com - 3.3.90-7 +- Remove changelog. The history is kept in Git, downstreams have own logs. +# note, this entry is here to placate tools that expect a non-empty changelog Can you please replace explicit version by the __VERSION__-__RELEASE__? Since this is a template file, __VERSION__-__RELEASE__ will get replaced by the latest version and release at the point of building the package. Sure. -- Petr³ From b5639f954f3032ae45463d346843c03ff1a02a6f Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Tue, 26 Nov 2013 13:06:07 +0100 Subject: [PATCH] Remove changelog from the spec The project's history is kept in Git. We used the spec changelog for changes to the spec itself, which doesn't make much sense. Downstreams like Fedora use their own changelog anyway. A single entry is left for tools that expect a changelog. --- freeipa.spec.in | 735 +--- 1 file changed, 3 insertions(+), 732 deletions(-) diff --git a/freeipa.spec.in b/freeipa.spec.in index 2a738dda1220615531f0a3f99137638cd9257111..35b87148c1074ae7e1e8909e981d3473c4a46258 100644 --- a/freeipa.spec.in +++ b/freeipa.spec.in @@ -839,735 +839,6 @@ fi %endif # ONLY_CLIENT %changelog -* Tue Nov 26 2013 Jan Cholasta jchol...@redhat.com - 3.3.90-6 -- Set minimum version of httpd to 2.4.6-6 -- Set minimum version of mod_nss to 1.0.8-26 - -* Tue Nov 12 2013 Tomas Babejtba...@redhat.com - 3.3.90-5 -- Add Fedora 19 platform files - -* Fri Oct 25 2013 Martin Kosek mko...@redhat.com - 3.3.90-4 -- Remove mod_ssl conflict, it can now live with mod_nss installed - -* Wed Sep 4 2013 Ana Krivokapic akriv...@redhat.com - 3.3.90-3 -- Conform to tmpfiles.d packaging guidelines - -* Wed Aug 14 2013 Petr Viktorin pvikt...@redhat.com - 3.3.90-2 -- Add man pages to the tests subpackage - -* Mon Aug 12 2013 Petr Viktorin pvikt...@redhat.com - 3.3.90-1 -- Downgrade required version of python-paramiko for the tests subpackage - -* Thu Aug 8 2013 Martin Kosek mko...@redhat.com - 3.2.99-13 -- Require slapi-nis 0.47.7 and sssd 1.11.0-0.1.beta2 required for core - features of 3.3.0 release - -* Fri Jul 26 2013 Martin Kosek mko...@redhat.com - 3.2.99-12 -- Require pki-ca 10.0.4 which fixes external CA installation (#986901) - -* Wed Jul 24 2013 Petr Viktorin pvikt...@redhat.com - 3.2.99-11 -- Add tar and xz dependencies to freeipa-tests - -* Wed Jul 24 2013 Tomas Babej tba...@redhat.com - 3.2.99-10 -- Move requirement for keyutils from freeipa-server to freeipa-python - -* Wed Jul 24 2013 Martin Kosek mko...@redhat.com - 3.2.99-9 -- Bump minimum version of sssd to 1.10.92 to pick up latest SSSD 1.11 Beta - development - -* Thu Jul 18 2013 Ana Krivokapic akriv...@redhat.com - 3.2.99-8 -- Bump minimum version of sssd to 1.10.90 for the 'ipa_server_mode' option. - -* Wed Jul 17 2013 Martin Kosek mko...@redhat.com - 3.2.99-7 -- Require selinux-policy 3.12.1-65 containing missing policy after removal of - freeipa-server-selinux subpackage - -* Tue Jul 16 2013 Tomas Babej tba...@redhat.com - 3.2.99-6 -- Do not create /var/lib/ipa/pki-ca/publish, retain reference as ghost - -* Thu Jul 11 2013 Martin Kosek mko...@redhat.com - 3.2.99-5 -- Run ipa-upgradeconfig and server restart in posttrans to avoid inconsistency - issues when there are still old parts of software (like entitlements plugin) - -* Wed Jul 10 2013 Ana Krivokapic akriv...@redhat.com - 3.2.99-4 -- Bump minimum version of 389-ds-base to 1.3.1.3 for user password change fix. - -* Wed Jun 26 2013 Jan Cholasta jchol...@redhat.com - 3.2.99-3 -- Bump minimum version of 389-ds-base to 1.3.1.1 for SASL mapping priority - support. - -* Mon Jun 17 2013 Petr Viktorin pvikt...@redhat.com - 3.2.99-2 -- Add the freeipa-tests subpackage - -* Thu Jun 13 2013 Martin Kosek mko...@redhat.com - 3.2.99-1 -- Drop freeipa-server-selinux subpackage -- Drop redundant directory /var/cache/ipa/sessions - -* Fri May 10 2013 Martin Kosek mko...@redhat.com - 3.1.99-13 -- Add requires for openldap-2.4.35-4 to pickup fixed SASL_NOCANON behavior for - socket based connections (#960222) - -* Tue May 7 2013 Petr Viktorin pvikt...@redhat.com - 3.1.99-12 -- Require libsss_nss_idmap-python in Fedora 19+ - -* Mon May 6 2013 Petr Vobornik pvobo...@redhat.com - 3.1.99-11 -- Web UI plugins
Re: [Freeipa-devel] [PATCH] 203 Remove mod_ssl port workaround
On 11/26/2013 12:17 PM, Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/4021. Honza I assume a build of httpd = 2.4.6-6 is not planned for Fedora 19, so master is now f20+ only. Is that right? -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 203 Remove mod_ssl port workaround
On 11/26/2013 02:15 PM, Alexander Bokovoy wrote: On Tue, 26 Nov 2013, Petr Viktorin wrote: On 11/26/2013 12:17 PM, Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/4021. Honza I assume a build of httpd = 2.4.6-6 is not planned for Fedora 19, so master is now f20+ only. Is that right? Is this bad? Given that we are close to F20 release, I'd prefer to concentrate on polishing and testing F20. Well, for me it means updating the infrastructure I use for development, including internal CI. It'll cost me some time, which I currently don't have a lot of. It would be nice to announce changes like this when sending the patch. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0132] [PATCH 132/132] trusts: Always stop and disable smb service on uninstall
On 11/22/2013 12:01 PM, Alexander Bokovoy wrote: On Thu, 21 Nov 2013, Tomas Babej wrote: https://fedorahosted.org/freeipa/ticket/4042 --- ipaserver/install/adtrustinstance.py | 15 +++ 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/ipaserver/install/adtrustinstance.py b/ipaserver/install/adtrustinstance.py index 5e3d0acbb11aae3c1a07512ec52b37fabcb0f644..2f1c99949969bd80ab14e6ae6c8145f53de17808 100644 --- a/ipaserver/install/adtrustinstance.py +++ b/ipaserver/install/adtrustinstance.py @@ -881,11 +881,16 @@ class ADTRUSTInstance(service.Service): if self.is_configured(): self.print_msg(Unconfiguring %s % self.service_name) -running = self.restore_state(running) -enabled = self.restore_state(enabled) +# Call restore_state so that we do not leave mess in the statestore +# Otherwise this does nothing +self.restore_state(running) +self.restore_state(enabled) +# Always try to stop and disable smb service, since we do not leave +# working configuration after uninstall try: self.stop() +self.disable() except: pass @@ -917,9 +922,3 @@ class ADTRUSTInstance(service.Service): # Remove our keys from samba's keytab self.clean_samba_keytab() - -if not enabled is None and not enabled: -self.disable() - -if not running is None and running: -self.start() ACK Pushed to master: d361e12ae55f391a13b613a7220c164f503954cc ipa-3-3: 6680572ad5c1419f094335c9f82a0e3763bf883e -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0321 Remove changelog from the spec
On 11/26/2013 01:45 PM, Alexander Bokovoy wrote: On Tue, 26 Nov 2013, Petr Viktorin wrote: On 11/26/2013 01:27 PM, Alexander Bokovoy wrote: On Tue, 26 Nov 2013, Petr Viktorin wrote: The changelog was useless and caused unnecessary rebase conflicts. Let's kill it. -- Petr³ From fe9d847fa39e3683ada3b7d12f3643ae9433bf45 Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Tue, 26 Nov 2013 13:06:07 +0100 Subject: [PATCH] Remove changelog from the spec The project's history is kept in Git. We used the spec changelog for changes to the spec itself, which doesn't make much sense. Downstreams like Fedora use their own changelog anyway. A single entry is left for tools that expect a changelog. ACK with one change: +* Tue Nov 26 2013 Petr Viktorinpvikt...@redhat.com - 3.3.90-7 +- Remove changelog. The history is kept in Git, downstreams have own logs. +# note, this entry is here to placate tools that expect a non-empty changelog Can you please replace explicit version by the __VERSION__-__RELEASE__? Since this is a template file, __VERSION__-__RELEASE__ will get replaced by the latest version and release at the point of building the package. Sure. ACK. Thanks! Thank you! Pushed to master: ba0da01c1d4eee25841aa0e19316d6953ff1bdea -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] #3901
On 11/26/2013 04:42 PM, Jan Cholasta wrote: On 26.11.2013 16:35, Jan Cholasta wrote: On 26.11.2013 14:24, Simo Sorce wrote: On Tue, 2013-11-26 at 14:11 +0100, Jan Cholasta wrote: kadmin.local still returns an error for me with this patch applied: kadmin.local: modprinc +ok_as_delegate host/test.example@example.com modify_principal: Kerberos database internal error while modifying host/test.example@example.com. I think I made a mistake using mod_op in ipadb_get_ldap_mod_str(), and should have used LDAP_MOD_ADD because we do not want to replace the objectclass object, we want to add to it. Any chance you can check quickly changing that ? I've blown the setup I was using when I created the patch That fixed it, so ACK with the change included. Modified patch attached. Thanks! Added ticket link to commit message and pushed to master: a1165ffbb80446890e3757113c9682c8526ed666 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0119 Switch client to JSON-RPC
On 11/26/2013 03:06 PM, Jan Cholasta wrote:
On 18.10.2013 12:26, Petr Viktorin wrote:
On 10/17/2013 06:08 PM, Jan Cholasta wrote:
Hi,
On 7.10.2013 18:16, Petr Viktorin wrote:
On 08/12/2013 10:17 AM, Petr Viktorin wrote:
On 08/02/2013 11:13 AM, Petr Viktorin wrote:
On 05/10/2013 04:54 PM, Petr Viktorin wrote:
On 04/01/2013 11:37 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
On 01/15/2013 12:36 PM, Petr Viktorin wrote:
I meant to hold this patch a while longer to let it mature, but
from
what Brian Smith asked on the user list it seems it could help
him.
Design: http://freeipa.org/page/V3/JSON-RPC
Ticket: https://fedorahosted.org/freeipa/ticket/3299
See the design page for what the patch does.
As much as I've tried to avoid them, the code includes some
workarounds:
It extends xmlrpclib to also support JSON. This is rather
intrusive,
but
to not do that I'd need to write a parallel stack for JSON,
without
the
help of a standard library.
It would be nice to write the JSON stack in the future anyway, this
indeed seems intrusive to me.
Yes, it would.
The registration of either jsonclient or xmlclient as
rpcclient in
the
API also needs a bit of magic, since the framework requires the
class
name to match the attribute.
You could also put the name of the plugin in a configuration option and
address the plugin like api.Backend[api.env.rpcclient_plugin], but I
guess your solution is no worse.
To prevent backwards compatibility problems, we need to ensure
that
all
official JSON clients send the API version, so this patch
should be
applied after my patches 0104-0106.
Updating to current master.
Please reverse this change in ipalib/rpc.py:
@@ -665,8 +788,6 @@ class xmlclient(Connectible):
except Exception, e:
if not fallback:
raise
-else:
-self.log.info('Connection to %s failed with
%s',
url, e)
serverproxy = None
This logs connection errors when the client fails over to another
server.
Thanks. Done, and rebased to master.
Thanks for the patch, it works for me.
I have just a few nitpicks:
def forward(self, *args, **kw):
-Forward call over XML-RPC to this same command on server.
+Forward call over JSON-RPC to this same command on server.
The new docstring is not entirely true.
Fixed, thanks
+def send_content(self, connection, request_body):
+if self.protocol == 'json':
+connection.putheader(Content-Type, application/json)
+else:
+connection.putheader(Content-Type, text/xml)
+
+connection.putheader(Content-Length, str(len(request_body)))
+connection.endheaders(request_body)
The original implementation of send_content in the Transport class sets
up gzip compression. I think it may be useful to do it here as well.
We don't opt in for gzip compression, so that's unnecessary.
I've added a comment.
+def rpc_uri_from_env(self, env):
+raise NotImplementedError('RPCClient.rpc_uri_from_env')
...
-xmlrpc_uri = self.env.xmlrpc_uri
+rpc_uri = self.rpc_uri_from_env(self.env)
I don't think this is necessary, since Env is also a mapping, you can do
this instead:
+env_rpc_uri_var = None
...
-xmlrpc_uri = self.env.xmlrpc_uri
+rpc_uri = self.env[env_rpc_uri_var]
You're right, changed
+class xmlclient(RPCClient):
+session_path = '/ipa/session/xml'
+server_proxy_class = ServerProxy
+protocol = 'xml'
+wrap_functions = xml_wrap, xml_unwrap
...
+class jsonclient(RPCClient):
+session_path = '/ipa/session/json'
+server_proxy_class = JSONServerProxy
+protocol = 'json'
+wrap_functions = identity, identity
It seems to me that json_encode_binary and json_decode_binary are
equivallent to xml_wrap and xml_unwrap, is there a reason you used the
identity function in jsonclient.wrap_functions?
Yes, it's for unwrapping error results. For JSON, we want the decoding
done before the error is raised, but for XML no decoding is done (error
results can't contain binary data).
I've moved this into a single method, hopefully it's clearer this way.
Thanks. ACK once you remove the unused identity function.
Thank you!
Removed the function and pushed to master:
73b8047b2298d347475a5c8d9f1853052ddced57
--
Petr³
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0016] Add RADIUS proxy support to ipalib CLI
On 11/21/2013 09:54 PM, Dmitri Pal wrote: On 11/21/2013 01:34 PM, Nathaniel McCallum wrote: The password can be retrieved with radiusproxy-show --all, because it is not blocked by LDAP ACIs. Is that intended? Yes. But I'm torn as to whether or not this is a good idea. Regular users can't see radius proxy servers at all. Admins can see all attributes. It is common in radius server deployments to have a text file readable by root with the radius secret. The current LDAP policy replicates this expected behavior. It may be wise to block all reads of the secret though. I'm open to suggestions. If it is readable by admin only I would leave it as is for now and address later when we redo ACIs. CCing Simo since this is ACI-related -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0016] Add RADIUS proxy support to ipalib CLI
Sorry for the late review!
On 11/21/2013 07:34 PM, Nathaniel McCallum wrote:
On Fri, 2013-11-15 at 12:34 +0100, Petr Viktorin wrote:
The password can be retrieved with radiusproxy-show --all, because it is
not blocked by LDAP ACIs. Is that intended?
Yes. But I'm torn as to whether or not this is a good idea. Regular
users can't see radius proxy servers at all. Admins can see all
attributes.
It is common in radius server deployments to have a text file readable
by root with the radius secret. The current LDAP policy replicates this
expected behavior. It may be wise to block all reads of the secret
though. I'm open to suggestions.
I'm fine either way, just making sure it gets some thought.
Let's see what Simo has to say on this.
ipatokenradiusserver is not validated. See validate_searchtimelimit in
the config plugin for an example validator. You can use validate_ipaddr
and validate_hostname from ipalib.util.
Fixed.
Now the validation is too strict, a port is not accepted.
Should non-FQDN hostnames be allowed?
ipatokenusermapattribute is also not validated. Not sure if it needs to be.
I don't think validation is really possible outside of the permitted
characters for an LDAP attribute.
I think if $%^* is allowed, we'll get a bug from QA soon enough.
The --secret CLI option does nothing (it doesn't take a value, and the
secret is prompted for whether or not the option is given). It should be
flagged no_option. (Or file an RFE for better Password semantics)
Fixed.
For the user commands, --radius takes the name on input, but a DN is
output. Is that expected?
Fixed.
We generally output lists; this should also be a list with one element.
Attaching updated tests.
--
Petr³
From a8145b2531222604e7883b298e00929727319a5a Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Fri, 15 Nov 2013 12:26:54 +0100
Subject: [PATCH] Add tests for the radiusproxy plugin
---
ipatests/test_xmlrpc/objectclasses.py | 5 +
ipatests/test_xmlrpc/test_radiusproxy_plugin.py | 384
2 files changed, 389 insertions(+)
create mode 100644 ipatests/test_xmlrpc/test_radiusproxy_plugin.py
diff --git a/ipatests/test_xmlrpc/objectclasses.py b/ipatests/test_xmlrpc/objectclasses.py
index 75ac3eb174aaa50eecfda875024b62dbdab238a5..089ee69a38b37f11de539e60b9ecacdec7a7de0b 100644
--- a/ipatests/test_xmlrpc/objectclasses.py
+++ b/ipatests/test_xmlrpc/objectclasses.py
@@ -161,3 +161,8 @@
u'nsContainer',
u'domainRelatedObject',
]
+
+radiusproxy = [
+u'ipatokenradiusconfiguration',
+u'top',
+]
diff --git a/ipatests/test_xmlrpc/test_radiusproxy_plugin.py b/ipatests/test_xmlrpc/test_radiusproxy_plugin.py
new file mode 100644
index ..4a10b393c8a1500c1bdf354f72b962cda6ab984a
--- /dev/null
+++ b/ipatests/test_xmlrpc/test_radiusproxy_plugin.py
@@ -0,0 +1,384 @@
+# 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 errors, api, _
+from ipapython.dn import DN
+from ipatests.test_xmlrpc.xmlrpc_test import Declarative
+from ipatests.test_xmlrpc.test_user_plugin import get_user_result
+from ipatests.test_xmlrpc import objectclasses
+
+radius1 = u'testradius'
+radius1_fqdn = u'testradius.test'
+radius1_dn = DN(('cn=testradius'), ('cn=radiusproxy'), api.env.basedn)
+user1 = u'tuser1'
+password1 = u'very*secure123'
+
+
+class test_raduisproxy(Declarative):
+
+cleanup_commands = [
+('radiusproxy_del', [radius1], {}),
+('user_del', [user1], {}),
+]
+
+tests = [
+
+dict(
+desc='Try to retrieve non-existent %r' % radius1,
+command=('radiusproxy_show', [radius1], {}),
+expected=errors.NotFound(
+reason=u'%s: RADIUS proxy server not found' % radius1),
+),
+
+
+dict(
+desc='Try to update non-existent %r' % radius1,
+command=('radiusproxy_mod', [radius1], {}),
+expected=errors.NotFound(
+reason=_('%s: RADIUS proxy server not found') % radius1),
+),
+
+
+dict(
+desc='Try to delete non-existent %r' % radius1,
+command=('radiusproxy_del', [radius1], {}),
+expected=errors.NotFound
Re: [Freeipa-devel] [PATCHES] 198-202 Refactor indirect membership processing
On 11/25/2013 03:27 PM, Jan Cholasta wrote:
On 8.11.2013 17:56, Petr Viktorin wrote:
Patch 198:
Also update ipaldap's find_entries docstring, it no longer uses IPA
defaults.
Done.
While you're touching this part of code, I had some other improvements
in mind -- you can consider them:
In find_entries,
attrs_list = [a.lower() for a in attrs_list]
to make sure 'memberindirect' is case insensitive
Done.
In get_memberof, construct `indirect` as a set, for Ο(1) remove().
^ ignore that, it's nuked in 201 \o/
Changing MEMBERS_ALL et.al. from numbers to descriptive strings, for
easier debugging.
^ these can be removed entirely in 201
Removed.
Patch 199: Looks great
Patch 200:
objtype, res_list, red_id, res_ctrls = result
Minor typo --^
Fixed.
This construction won't work as you'd expect in Python 2:
try:
(possibly raise interesting exception) (*)
except:
try:
(possibly raise exception to ignore) (**)
except:
pass
raise # (***)
The problem is that the exception in (**) overwrites the current active
exception raised in (*). In (***) the exception from the cleanup
will be
re-raised.
The solution is to store the wanted exception info, including the
traceback:
exc_type, exc_value, exc_traceback = sys.exc_info()
and then re-raise explicitly:
raise exc_type, exc_value, exc_traceback
Turns out LDAPError does not fly well with sys.exc_info() (all exception
data is lost, probably a python-ldap bug), so I used except LDAPError,
e: ... raise e instead.
Also, please log the ignored exception from cancelling the paged search.
Done.
Patch 201:
Great patch!
A nitpick, I'd rename _process_member{,of} to
_process_member{,of}indirect
Renamed.
Patch 202: Looks good
While we're on the subject: Each Plugin has an api attribute. It would
be nice if we started preferring `self.api` instead of the global
singleton wherever possible, even though they're currently always the
same.
+1, fixed.
Not fixed, but it's okay for now.
Updated patches attached.
Honza
Thanks! ACK, pushed to master: 4050e553c30d8c8d93c7045f871f8c1cef65aa71
--
Petr³
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 203 Remove mod_ssl port workaround
On 11/26/2013 02:35 PM, Alexander Bokovoy wrote: On Tue, 26 Nov 2013, Petr Viktorin wrote: On 11/26/2013 02:15 PM, Alexander Bokovoy wrote: On Tue, 26 Nov 2013, Petr Viktorin wrote: On 11/26/2013 12:17 PM, Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/4021. Honza I assume a build of httpd = 2.4.6-6 is not planned for Fedora 19, so master is now f20+ only. Is that right? Is this bad? Given that we are close to F20 release, I'd prefer to concentrate on polishing and testing F20. Well, for me it means updating the infrastructure I use for development, including internal CI. It'll cost me some time, which I currently don't have a lot of. Could we switch CI to track 3.3 branch for pre-F20? I just realized we have a problem here: this patch also went to ipa-3-3. That means 3.3 is currently also f20+ only. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [RFE] Permissions V2
On 11/11/2013 04:48 PM, Petr Viktorin wrote: On 11/11/2013 03:56 PM, Rob Crittenden wrote: Petr Viktorin wrote: Hello, I'm splitting up ACI work into several designs to make it more manageable. This one is about - Moving ACIs out of $SUFFIX - Storing all ACI data in the permission entry - Permission flag system for ensuring backwards compatibility Summary of the backcompat story: - Attributes, rights, etc. of new permissions may not be modified or read on old servers (not possible since the ACIs aren't in $SUFFIX) - Old permissions convert to new ones when they're modified on a new server - Any server can assign (or remove) both old and new permissions to privileges There is a bit of shuffling in API/CLI option names, since the API option name needs to match the LDAP attributeTypes. The WIP design document is here: http://www.freeipa.org/page/V3/Permissions_V2 I've updated the design with - updated schema (this time the OIDs are even reserved properly!) - longer attribute descriptions with examples - updated update algorithm based on discussion with Simo Additionally, I've updated draft designs this one references [0, 1]. The CLI/API parts of those aren't finished but the LDAP should be ready for criticism. For examples, I felt that anything I show as an example should also go in the test suite, so I added the tests. (If you're into wiki design I'd appreciate ideas about how to make that section better.) If you need any more examples, or see some dangerous corner cases, tell me and I'll add them. There is still a race condition when the subtree changes, e.g. when you'd move an ACI from $SUFFIX to cn=users,cn=accounts,$SUFFIX, the rights are revoked between the times the ACI is removed and re-added. At this point I'd rather document it and file a bug (and possibly start working on it right after this) than redo the internals in yet another way in the same update. [0] http://www.freeipa.org/page/V3/Anonymous_and_All_permissions [1] http://www.freeipa.org/page/V3/Managed_Read_permissions PS. the hack I used to generate the test plan for mediawiki is here: https://github.com/encukou/ipa-tools/blob/master/mw-format-tests.py Data in the permission entry I think the schema needs to be described better. I'm assuming that ipaPermTarget is the equivalent of current targetgroup option? And targetfilter is the equivalent of current filter option? ipaPermTarget is the raw ACI target, i.e. the DN. targetgroup is just the name If the targetgroup option is specified, it effectively just finds the group and sets the ipaPermTarget option to its DN. And if ipaPermTarget contains a group DN, targetgroup will be populated with the cn on output (in addition to ipaPermTarget with the full DN). The next update will have examples. If you are placing the ACI into the container based on type, then you probably don't need the filter within the ACI (it is implicit). Sorry; that should have been target, not fiter. The target is a bit more explicit; it has e.g. uid=* in addition to the user container DN, so I'd like to keep both. In general I think some examples would be helpful. I'll add some. Modifying and Upgrading Permissions Under what conditions would there be an old or a new permission and no associated ACI? If a command failed unexpectedly, and also failed to clean up. For example if the DS shuts down at the right time in the middle of a permission-mod. Option/Attribute mapping Performing a search on the filter is a good idea but realize that it has its limits. It is possible to create a legal filter that makes no (or little) sense. Of course. This is just a syntax check. If type is only going to specify the location of the ACI then perhaps it shouldn't be in the mutually exclusive list. Yes. Martin just pointed out ticket 2355_ (Allow filter and subtree to be added in same permission) to me today. I'll redo the mutual exclusivity so more things are possible together. _2355 :https://fedorahosted.org/freeipa/ticket/2355 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 204-205 Spec file fixes
On 11/27/2013 02:50 PM, Martin Kosek wrote:
On 11/27/2013 02:26 PM, Jan Cholasta wrote:
Hi,
the attached patches fix https://fedorahosted.org/freeipa/ticket/4010.
This fixes points 2) 3) in the ticket; point 1) is not applicable; 4)
are false positives.
The checks mentioned in the ticket pass.
$ hardening-check --color --verbose /usr/libexec/ipa-otpd
/usr/libexec/ipa-otpd:
Position Independent Executable: yes
Stack protected: yes
Fortify Source functions: yes (some protected functions found)
unprotected: gethostname
unprotected: read
protected: vfprintf
protected: asprintf
protected: memcpy
protected: fprintf
Read-only relocations: yes
Immediate binding: yes
pviktori@vm-183:~/freeipa{master}16e60f7$ readelf -d
/usr/libexec/ipa-otpd | grep BIND_NOW
0x0018 (BIND_NOW)
pviktori@vm-183:~/freeipa{master}16e60f7$ readelf -h
/usr/libexec/ipa-otpd | grep Type
Type: DYN (Shared object file)
(Note, redhat-rpm-config is part of Fedora's minimal build environment:
https://fedoraproject.org/wiki/Packaging:Guidelines#Exceptions_2)
Honza
Do we want to define
+%if (0%{?fedora} 15 || 0%{?rhel} = 7)
+%define _hardened_build 1
+%endif
globally? Wouldn't it trigger the hardening also for all our C utilities or
internal SLAPI plugins? Wouldn't it have performance implication for the SLAPI
plugins?
I am not sure, I would like to hear what the experts say.
Martin
On 11/27/2013 03:37 PM, Jakub Hrozek wrote: I'm sorry, I removed
Martin's e-mail by accident so I'll reply here. I
think defining the hardened build globally is fine, the only performance
impact is during startup and only small.
AFAIR, the C utilities in IPA are mostly daemons and you really want to
have full RELRO enabled there.
The only gotcha we found so far (well, Nalin did) was that SELinux was
not happy with full RELRO on some exotic architectures, like s390x
Is that a SELinux bug? Should we care about it?
--
Petr³
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 203 Remove mod_ssl port workaround
On 11/29/2013 03:50 PM, Alexander Bokovoy wrote: On Fri, 29 Nov 2013, Martin Kosek wrote: On 11/29/2013 03:30 PM, Petr Viktorin wrote: On 11/26/2013 02:35 PM, Alexander Bokovoy wrote: On Tue, 26 Nov 2013, Petr Viktorin wrote: On 11/26/2013 02:15 PM, Alexander Bokovoy wrote: On Tue, 26 Nov 2013, Petr Viktorin wrote: On 11/26/2013 12:17 PM, Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/4021. Honza I assume a build of httpd = 2.4.6-6 is not planned for Fedora 19, so master is now f20+ only. Is that right? Is this bad? Given that we are close to F20 release, I'd prefer to concentrate on polishing and testing F20. Well, for me it means updating the infrastructure I use for development, including internal CI. It'll cost me some time, which I currently don't have a lot of. Could we switch CI to track 3.3 branch for pre-F20? I just realized we have a problem here: this patch also went to ipa-3-3. That means 3.3 is currently also f20+ only. I see two options here: 1) Update the CI FreeIPA build instruction and remove the F20 httpd Requires. All tests should still work as long as mod_ssl is not installed. 3.3 shouldn't need this require, so we should back out the patch from there. That makes sense. Reverted in ipa-3-3: c6a15335b0406c9d7d57378cbdd9b20252438f65 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 439 Allow kernel keyring CCACHE when supported
On 11/29/2013 01:48 PM, Martin Kosek wrote: On 11/19/2013 12:35 PM, Petr Viktorin wrote: On 11/05/2013 07:22 PM, Martin Kosek wrote: Server and client installer should allow kernel keyring ccache when supported. How do I enable the kernel keyring? On f20 I get this: 2013-11-19T11:28:07Z DEBUG Starting external process 2013-11-19T11:28:07Z DEBUG args=keyctl get_persistent @s 0 2013-11-19T11:28:07Z DEBUG Process finished, return code=1 2013-11-19T11:28:07Z DEBUG stdout= 2013-11-19T11:28:07Z DEBUG stderr=keyctl_get_persistent: Key has been revoked It should be enabled out of the box. But there were some initial issues with persistent keyring in the first versions of kernel with a support, hopefully this was just a fluke which disappeared. This is what I see on my F20 with kernel-3.11.9-300.fc20.x86_64: # keyctl get_persistent @s 0 637466038 With kernel-3.11.10-300.fc20.x86_64, I get an error again: $ keyctl get_persistent @s 0 keyctl_get_persistent: Key has been revoked I don't know much about the kernel keyring, so I'm lost as to what the message is trying to tell me. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [RFE] Permissions V2
On 12/02/2013 02:29 PM, Simo Sorce wrote: On Fri, 2013-11-29 at 16:51 +0100, Petr Viktorin wrote: I've updated the design with - updated schema (this time the OIDs are even reserved properly!) - longer attribute descriptions with examples - updated update algorithm based on discussion with Simo Hi Petr, thank you for the update. Additionally, I've updated draft designs this one references [0, 1]. The CLI/API parts of those aren't finished but the LDAP should be ready for criticism. It would be very nice if you can add the resulting LDAP objects in the example, that will allow me to reason on the correctness of the translation. OK, I'll work on that. For examples, I felt that anything I show as an example should also go in the test suite, so I added the tests. (If you're into wiki design I'd appreciate ideas about how to make that section better.) If you need any more examples, or see some dangerous corner cases, tell me and I'll add them. There is still a race condition when the subtree changes, e.g. when you'd move an ACI from $SUFFIX to cn=users,cn=accounts,$SUFFIX, the rights are revoked between the times the ACI is removed and re-added. At this point I'd rather document it and file a bug (and possibly start working on it right after this) than redo the internals in yet another way in the same update. I think that this will be fine, *after* we change the default mode to deny everything, and rely on permissions to allow. This way the lack of an ACI will deny (not permit!) access to arbitrary attributes. Permissions can only allow access. All our deny ACIs are built in, not controlled by permissions. [0] http://www.freeipa.org/page/V3/Anonymous_and_All_permissions [1] http://www.freeipa.org/page/V3/Managed_Read_permissions -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 439 Allow kernel keyring CCACHE when supported
On 12/02/2013 02:01 PM, Martin Kosek wrote: On 12/02/2013 01:58 PM, Petr Viktorin wrote: On 11/29/2013 01:48 PM, Martin Kosek wrote: On 11/19/2013 12:35 PM, Petr Viktorin wrote: On 11/05/2013 07:22 PM, Martin Kosek wrote: Server and client installer should allow kernel keyring ccache when supported. How do I enable the kernel keyring? On f20 I get this: 2013-11-19T11:28:07Z DEBUG Starting external process 2013-11-19T11:28:07Z DEBUG args=keyctl get_persistent @s 0 2013-11-19T11:28:07Z DEBUG Process finished, return code=1 2013-11-19T11:28:07Z DEBUG stdout= 2013-11-19T11:28:07Z DEBUG stderr=keyctl_get_persistent: Key has been revoked It should be enabled out of the box. But there were some initial issues with persistent keyring in the first versions of kernel with a support, hopefully this was just a fluke which disappeared. This is what I see on my F20 with kernel-3.11.9-300.fc20.x86_64: # keyctl get_persistent @s 0 637466038 With kernel-3.11.10-300.fc20.x86_64, I get an error again: $ keyctl get_persistent @s 0 keyctl_get_persistent: Key has been revoked Not sure if it is a typo, but you won't surely get a root's keyring as a non-root user... It is just a typo, but it looks like you got me on the right track. keyctl apparently needs a real root login: $ sudo keyctl get_persistent @s 0 keyctl_get_persistent: Key has been revoked $ sudo su # keyctl get_persistent @s 0 keyctl_get_persistent: Key has been revoked # exit $ sudo su - Last login: Mon Dec 2 14:09:36 CET 2013 on pts/1 # keyctl get_persistent @s 0 968622527 # logout Unsurprisingly, when ipa-server-install is run from sudo, it complains that the key is unsupported. From a root login all is OK. Is that expected? -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 439 Allow kernel keyring CCACHE when supported
On 12/02/2013 03:42 PM, Simo Sorce wrote: On Mon, 2013-12-02 at 14:51 +0100, Petr Viktorin wrote: On 12/02/2013 02:01 PM, Martin Kosek wrote: On 12/02/2013 01:58 PM, Petr Viktorin wrote: On 11/29/2013 01:48 PM, Martin Kosek wrote: On 11/19/2013 12:35 PM, Petr Viktorin wrote: On 11/05/2013 07:22 PM, Martin Kosek wrote: Server and client installer should allow kernel keyring ccache when supported. How do I enable the kernel keyring? On f20 I get this: 2013-11-19T11:28:07Z DEBUG Starting external process 2013-11-19T11:28:07Z DEBUG args=keyctl get_persistent @s 0 2013-11-19T11:28:07Z DEBUG Process finished, return code=1 2013-11-19T11:28:07Z DEBUG stdout= 2013-11-19T11:28:07Z DEBUG stderr=keyctl_get_persistent: Key has been revoked It should be enabled out of the box. But there were some initial issues with persistent keyring in the first versions of kernel with a support, hopefully this was just a fluke which disappeared. This is what I see on my F20 with kernel-3.11.9-300.fc20.x86_64: # keyctl get_persistent @s 0 637466038 With kernel-3.11.10-300.fc20.x86_64, I get an error again: $ keyctl get_persistent @s 0 keyctl_get_persistent: Key has been revoked Not sure if it is a typo, but you won't surely get a root's keyring as a non-root user... It is just a typo, but it looks like you got me on the right track. keyctl apparently needs a real root login: $ sudo keyctl get_persistent @s 0 keyctl_get_persistent: Key has been revoked $ sudo su # keyctl get_persistent @s 0 keyctl_get_persistent: Key has been revoked # exit $ sudo su - Last login: Mon Dec 2 14:09:36 CET 2013 on pts/1 # keyctl get_persistent @s 0 968622527 # logout Please use sudo -i to get an interactive 'login' shell. Unsurprisingly, when ipa-server-install is run from sudo, it complains that the key is unsupported. From a root login all is OK. Is that expected? You should run ipa-server-install using a login shell I think. Should we open a bug to detect this and fail ? It's always worked with just sudo for me. So yes, if it's required I think we should enforce it. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCHES] 0328-0329 test_integration: Support external names for hosts
Hello,
Patch 0328 fixes the problem that I filed [4038] for: CA-less tests fail
when run in some setups on Beaker. What the ticket says was premature
diagnosis; I'll close the ticket as invalid.
See commit message for the real problem.
Patch 0329 adds a bit more info to logs.
[4038] https://fedorahosted.org/freeipa/ticket/4038 tests: Forwarder is
not set on replicas
--
Petr³
From 5c3bb59c6c0a46e2841a95d643edf29692024e4e Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Thu, 24 Oct 2013 12:14:58 +0200
Subject: [PATCH] test_integration: Support external names for hosts
The framework had a concept of external hostnames,
which the controller uses to contact the test machines,
but they were not loaded from configuration.
Load external names from configuration.
This makes tests pass in setups where internal and external
hostnames are different, and the internal hostnames are not
initially resolvable from the controller.
---
ipatests/test_integration/config.py | 14 ++
ipatests/test_integration/host.py | 9 ++---
2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/ipatests/test_integration/config.py b/ipatests/test_integration/config.py
index 3aa4d05d6cb5758cd0d6be64a1ac582adcc971b4..b8c5fdc7f9ce1877e34491964418a8d806168e73 100644
--- a/ipatests/test_integration/config.py
+++ b/ipatests/test_integration/config.py
@@ -236,8 +236,10 @@ def env_normalize(env):
Fill env variables from alternate variable names
MASTER_env1 - MASTER
-REPLICA_env1 - REPLICA
-CLIENT_env1 - CLIENT, SLAVE
+REPLICA_env1 - REPLICA, SLAVE
+CLIENT_env1 - CLIENT
+similarly for BEAKER* variants: BEAKERMASTER1_env1 - BEAKERMASTER, etc.
+
CLIENT_env1 gets extended with CLIENT2 or CLIENT2_env1
def coalesce(name, *other_names):
@@ -253,8 +255,12 @@ def coalesce(name, *other_names):
else:
env[name] = ''
coalesce('MASTER_env1', 'MASTER')
-coalesce('REPLICA_env1', 'REPLICA')
-coalesce('CLIENT_env1', 'CLIENT', 'SLAVE')
+coalesce('REPLICA_env1', 'REPLICA', 'SLAVE')
+coalesce('CLIENT_env1', 'CLIENT')
+
+coalesce('BEAKERMASTER1_env1', 'BEAKERMASTER')
+coalesce('BEAKERREPLICA1_env1', 'BEAKERREPLICA', 'BEAKERSLAVE')
+coalesce('BEAKERCLIENT1_env1', 'BEAKERCLIENT')
def extend(name, name2):
value = env.get(name2)
diff --git a/ipatests/test_integration/host.py b/ipatests/test_integration/host.py
index 02c82b372ce2805c0ca922319f5de1cd29b0ed82..be45a0337a9fe15b3463e839b324daa1effdf8e6 100644
--- a/ipatests/test_integration/host.py
+++ b/ipatests/test_integration/host.py
@@ -32,7 +32,8 @@ class BaseHost(object):
Representation of a remote IPA host
transport_class = None
-def __init__(self, domain, hostname, role, index, ip=None):
+def __init__(self, domain, hostname, role, index, ip=None,
+ external_hostname=None):
self.domain = domain
self.role = role
self.index = index
@@ -40,7 +41,7 @@ def __init__(self, domain, hostname, role, index, ip=None):
shortname, dot, ext_domain = hostname.partition('.')
self.shortname = shortname
self.hostname = shortname + '.' + self.domain.name
-self.external_hostname = hostname
+self.external_hostname = external_hostname or hostname
self.netbios = self.domain.name.split('.')[0].upper()
@@ -96,6 +97,8 @@ def remove_log_collector(self, collector):
def from_env(cls, env, domain, hostname, role, index):
ip = env.get('BEAKER%s%s_IP_env%s' %
(role.upper(), index, domain.index), None)
+external_hostname = env.get(
+'BEAKER%s%s_env%s' % (role.upper(), index, domain.index), None)
# We need to determine the type of the host, this depends on the domain
# type, as we assume all Unix machines are in the Unix domain and
@@ -106,7 +109,7 @@ def from_env(cls, env, domain, hostname, role, index):
else:
cls = Host
-self = cls(domain, hostname, role, index, ip)
+self = cls(domain, hostname, role, index, ip, external_hostname)
return self
@property
--
1.8.3.1
From 23cfbd130e08c87ac5804aad7e7a031a124cb8a5 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Thu, 21 Nov 2013 13:57:47 +0100
Subject: [PATCH] test_integration: Log external hostname in Host.ldap_connect
This may make debugging easier if the address is set incorrectly.
---
ipatests/test_integration/host.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ipatests/test_integration/host.py b/ipatests/test_integration/host.py
index be45a0337a9fe15b3463e839b324daa1effdf8e6..507e19ed62b3d0a76e6e2ff6286fd83f17a68627 100644
--- a/ipatests/test_integration/host.py
+++ b/ipatests/test_integration/host.py
@@ -160,7 +160,7 @@ def put_file_contents(self, filename, contents):
def ldap_connect(self):
Return
Re: [Freeipa-devel] [PATCH 0016] Add RADIUS proxy support to ipalib CLI
On 11/28/2013 04:59 PM, Nathaniel McCallum wrote: Everything looks good to me. +1 Pushed to master: a1f32fa9369109235dba041de9c972da09d8448a -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 206-209 Add default CFLAGS fix hardened build
On 12/05/2013 11:15 AM, Jan Cholasta wrote: Hi, the attached patches fix https://fedorahosted.org/freeipa/ticket/3896. Patch 207 should fix build failures some of you were having after hardenening was enabled in the spec file. Thanks! In 209, would (ret != 1) make more sense than (ret == -1)? AFAIU zero would also indicate a problem, if it somehow ended up being returned. It seems though the hardening flags get included twice. I assume that's not a problem? I get lines like: gcc -DHAVE_CONFIG_H -I. -I.. -I/usr/include/nspr4 -I/usr/include/nss3 -I/usr/include/nspr4 -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic -g -O2 -Werror -Wall -Wextra -Wformat-security -Wno-unused-parameter -Wno-sign-compare -Wno-missing-field-initializers -DWITH_OPENLDAP -I/usr/include/nspr4 -I/usr/include/nss3 -I/usr/include/nspr4 -DUSE_OPENLDAP -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic -g -O2 -Werror -Wall -Wextra -Wformat-security -Wno-unused-parameter -Wno-sign-compare -Wno-missing-field-initializers -MT parse.o -MD -MP -MF .deps/parse.Tpo -c -o parse.o parse.c gcc -DHAVE_CONFIG_H -I. -I. -I. -I../util -DPREFIX=\/usr\ -DBINDIR=\/usr/bin\ -DLIBDIR=\/usr/lib64\ -DLIBEXECDIR=\/usr/libexec\ -DDATADIR=\/usr/share\ -DLOCALEDIR=\/usr/share/locale\ -Wall -Wshadow -Wstrict-prototypes -Wpointer-arith -Wcast-align -Werror-implicit-function-declaration -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic -Wall -Wshadow -Wstrict-prototypes -Wpointer-arith -Wcast-align -Werror-implicit-function-declaration -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic -g -O2 -Werror -Wall -Wextra -Wformat-security -Wno-unused-parameter -Wno-sign-compare -Wno-missing-field-initializers -MT ipa-getkeytab.o -MD -MP -MF .deps/ipa-getkeytab.Tpo -c -o ipa-getkeytab.o ipa-getkeytab.c -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 0330 - Add comment about last change to VERSION
Consider this scenario: - Nathaniel submits RADIUS patches that update the API version (from 2.69 to 2.70) - I have ACI patches that also bump the version (from 2.69 to 2.70) - Nathaniel's patches gets accepted - I rebase my ACI patches onto master. Git thinks that the 2.69-2.70 change is already done, so it leaves VERSION unchanged. I can solve this locally by telling Git to not merge VERSION automatically, but I think it would be helpful to add a unique comment to each change so that everyone gets a conflict cases like this. Do you agree? -- Petr³ From 064acd3c1ef7524c2525fb9266ff5fe3251d23d3 Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Thu, 5 Dec 2013 13:31:19 +0100 Subject: [PATCH] Add comment about last change to VERSION When a branch with API version bump is rebased, but the version was also bumped in master, Git thinks the change was already done and loses it. A comment unique to each change will cause a merge conflict in this case, so the developer is reminded to update the number. --- VERSION | 1 + 1 file changed, 1 insertion(+) diff --git a/VERSION b/VERSION index e7d7bc3eab38c57cd9c5b24f13c27a234b9c6f03..694f639d03bf9d02dc577b20358b6018609132d1 100644 --- a/VERSION +++ b/VERSION @@ -90,3 +90,4 @@ IPA_DATA_VERSION=2010061412 IPA_API_VERSION_MAJOR=2 IPA_API_VERSION_MINOR=70 +# Last change: npmccallum - RADIUS support -- 1.8.3.1 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Fix python setup tools license tags
On 12/03/2013 03:26 PM, Simo Sorce wrote:
Some tags escaped the relicensing we did a long time ago.
Simo.
Looks good, ACK, pushed to:
master: af26e6da4650b3a429af31bc38b546eff27e38c6
ipa-3-3: 9defb913aa65bfe9b423d510f340ae23b9e547f2
I grepped for some other occurences of GPLv2:
contrib/RHEL4/ipa-client.spec:7:License:GPLv2
do we still want to carry the RHEL4 stuff anyway?
ipa-client/ipa-client.spec.in:7:License:GPLv2
Is ipa-client.spec used for anything any more?
install/ui/src/freeipa/package.json:
licenses: [{
type: GPLv3,
url: http://www.gnu.org/licenses/gpl-3.0.html;
},{
type: GPLv2,
url: http://www.gnu.org/licenses/gpl-2.0.html;
}],
Is this package dual-licensed?
--
Petr³
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Fix python setup tools license tags
On 12/05/2013 04:02 PM, Simo Sorce wrote:
On Thu, 2013-12-05 at 15:38 +0100, Petr Vobornik wrote:
On 5.12.2013 15:34, Simo Sorce wrote:
On Thu, 2013-12-05 at 15:29 +0100, Petr Vobornik wrote:
On 5.12.2013 14:09, Petr Viktorin wrote:
On 12/03/2013 03:26 PM, Simo Sorce wrote:
Some tags escaped the relicensing we did a long time ago.
Simo.
Looks good, ACK, pushed to:
master: af26e6da4650b3a429af31bc38b546eff27e38c6
ipa-3-3: 9defb913aa65bfe9b423d510f340ae23b9e547f2
I grepped for some other occurences of GPLv2:
contrib/RHEL4/ipa-client.spec:7:License:GPLv2
do we still want to carry the RHEL4 stuff anyway?
ipa-client/ipa-client.spec.in:7:License:GPLv2
Is ipa-client.spec used for anything any more?
install/ui/src/freeipa/package.json:
licenses: [{
type: GPLv3,
url: http://www.gnu.org/licenses/gpl-3.0.html;
},{
type: GPLv2,
url: http://www.gnu.org/licenses/gpl-2.0.html;
}],
Is this package dual-licensed?
It's because of:
git grep Free Software Foundation; version 2
install/ui/src/freeipa/aci.js: * published by the Free Software
Foundation; version 2 only
install/ui/test/aci_tests.js: * published by the Free Software
Foundation; version 2 only
install/ui/test/widget_tests.js: * published by the Free Software
Foundation; version 2 only
It's most likely a mistake and should be changed.
Is that code really v2 only ?
Or are you saying the version 2 only strings are mistakes ?
Simo.
It's our code. So IMO we should just change it to v3.
I do not recall we ever used the v2 only variant, this is highly
suspect, we should go through history and make sure it is all our code,
then re-license it.
If it is derived from v2 only code from an outside party though then we
will need to ask for permission to change or strip the code out and
rewrite it from scratch.
Can someone check through git history and determine where the code comes
from and how the only label got onto it ?
There were Red Hat¹ contributors only so far:
$ for file in
install/ui/{src/freeipa/aci.js,test/aci_tests.js,test/widget_tests.js};
do git log --follow --raw $file; done | grep ^Author: | sort | uniq
Author: Adam Young ayo...@redhat.com
Author: Endi S. Dewata edew...@redhat.com
Author: Endi Sukma Dewata edew...@redhat.com
Author: Martin Kosek mko...@redhat.com
Author: Petr Vobornik pvobo...@redhat.com
Author: Petr Voborník pvobo...@redhat.com
The files come from these commits, with the only label already in them:
c281e786c805f400ca23d4412e29d396632d5441 widget unit tests
07ace112afeaadade0ca372fe23a9432c2c9780f aci ui
or without tracking renames:
b9ef6ab0c412913234f05f788b3fcd3c3277eb69 Move of core Web UI files to
AMD directory
b9ad279ad2d8d93dd501115a028783cf8fe7fcbd rename static to ui
c281e786c805f400ca23d4412e29d396632d5441 widget unit tests
--
Petr³
¹ I object to using we to mean Red Hat on this list.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 206-209 Add default CFLAGS fix hardened build
On 12/06/2013 11:52 AM, Jan Cholasta wrote: On 5.12.2013 13:31, Alexander Bokovoy wrote: On Thu, 05 Dec 2013, Petr Viktorin wrote: On 12/05/2013 11:15 AM, Jan Cholasta wrote: Hi, the attached patches fix https://fedorahosted.org/freeipa/ticket/3896. Patch 207 should fix build failures some of you were having after hardenening was enabled in the spec file. Thanks! In 209, would (ret != 1) make more sense than (ret == -1)? AFAIU zero would also indicate a problem, if it somehow ended up being returned. no, write() returns -1 if an error has happened and amount of the data written otherwise. We specifically need to check that there was an error, not that we wrote more or less than were supposed to write. We are looking for EPIPE and EINTR mostly, since other errors make little difference for our case. In case of EINTR we need to repeat or the worker thread didn't receive our shutdown request. In case of EPIPE we will also get SIGPIPE and in general this means the other thread died.. However, even if writing to the pipe failed, we still need to wait until thread dies with pthread_join(). I think returning -1 here is premature. Fixed, updated patches attached. Also removed CFLAGS duplication, see patch 212. Thanks you! The build works fine, ACK for 206-208, 212. Alexander, is the C part OK? It seems a do-while loop would make sense here. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [RFE] Permissions V2
On 12/02/2013 02:48 PM, Petr Viktorin wrote: On 12/02/2013 02:29 PM, Simo Sorce wrote: On Fri, 2013-11-29 at 16:51 +0100, Petr Viktorin wrote: I've updated the design with - updated schema (this time the OIDs are even reserved properly!) - longer attribute descriptions with examples - updated update algorithm based on discussion with Simo Hi Petr, thank you for the update. Additionally, I've updated draft designs this one references [0, 1]. The CLI/API parts of those aren't finished but the LDAP should be ready for criticism. It would be very nice if you can add the resulting LDAP objects in the example, that will allow me to reason on the correctness of the translation. OK, I'll work on that. I've added the resulting LDAP objects to the tests here: http://www.freeipa.org/index.php?title=V3/Permissions_V2/tests For examples, I felt that anything I show as an example should also go in the test suite, so I added the tests. (If you're into wiki design I'd appreciate ideas about how to make that section better.) If you need any more examples, or see some dangerous corner cases, tell me and I'll add them. There is still a race condition when the subtree changes, e.g. when you'd move an ACI from $SUFFIX to cn=users,cn=accounts,$SUFFIX, the rights are revoked between the times the ACI is removed and re-added. At this point I'd rather document it and file a bug (and possibly start working on it right after this) than redo the internals in yet another way in the same update. I think that this will be fine, *after* we change the default mode to deny everything, and rely on permissions to allow. This way the lack of an ACI will deny (not permit!) access to arbitrary attributes. Permissions can only allow access. All our deny ACIs are built in, not controlled by permissions. [0] http://www.freeipa.org/page/V3/Anonymous_and_All_permissions [1] http://www.freeipa.org/page/V3/Managed_Read_permissions -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 206-209 Add default CFLAGS fix hardened build
On 12/06/2013 02:26 PM, Alexander Bokovoy wrote:
On Fri, 06 Dec 2013, Jan Cholasta wrote:
However, even if writing to the pipe failed, we still need to wait
until
thread dies with pthread_join(). I think returning -1 here is
premature.
Fixed, updated patches attached.
Also removed CFLAGS duplication, see patch 212.
Thanks you! The build works fine, ACK for 206-208, 212.
Alexander, is the C part OK? It seems a do-while loop would make sense
here.
I think do-while would be cleaner, purely from intent expression point
of view.
I actually like it the way it is, because it follows the
ret = func()
if (ret == -1) {
}
pattern, which is used abundantly in our code.
... but I don't have a strong opinion about this, so whatever floats
your boat.
Thanks. It is just that in this case -write(ctx-stopfd[1], , 1);
+do {
+ret = write(ctx-stopfd[1], , 1);
+} while (ret == -1 errno == EINTR);
makes more clear that we loop here only for EINTR.
ACK from my side
Thank you both, pushed to master: 5e2f7b68f0cb8e7fd6ea4f3236e84f1a8d075a13
--
Petr³
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] FreeIPA Continuous Integration Configuration
Hello, As some of you are aware, I'm running a Jenkins instance for FreeIPA continuous integration in our lab here at Red Hat Brno. I'm currently porting the job definitions to jenkins-job-builder[0] for ease of management. This allowed me to strip out the private bits from the configuration, so anyone can use the config to run FreeIPA tests in their own Jenkins. In the spirit of release early, here is the repo: https://github.com/encukou/freeipa-ci I'll update as I port more of the tests to YAML. If you want any help setting up tests for IPA, or if you have some suggestions, please get in touch! [0] https://github.com/openstack-infra/jenkins-job-builder -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [RFE] Permissions V2
On 12/06/2013 03:28 PM, Simo Sorce wrote: On Fri, 2013-12-06 at 14:14 +0100, Petr Viktorin wrote: On 12/02/2013 02:48 PM, Petr Viktorin wrote: On 12/02/2013 02:29 PM, Simo Sorce wrote: It would be very nice if you can add the resulting LDAP objects in the example, that will allow me to reason on the correctness of the translation. OK, I'll work on that. I've added the resulting LDAP objects to the tests here: http://www.freeipa.org/index.php?title=V3/Permissions_V2/tests Thank you Petr, I was looking at them and I see we often use target=ldap://dn type for selecting which objects this apply to. This was sort of necessary when the permissions were all in the base and we wanted to limit to specific entries in subtrees. However I was wondering if we shouldn't transition/allow to user targetfilter or targetattrfilter (this would be needed to have add/delete permissions). For example, instead of: (target = ldap:///uid=*,cn=users,cn=accounts,$SUFFIX;) We could have: (targetfilter = (objectclass=ipaUser)) It also occurs to me we could do very neat things like allowing manager access with (targetfilter = (managedby=dn)), and similar. In general using targetfilter and targetattrfilter is more flexible and allow for applying different permission depending exacly on the object type or even specific sets of objects of a common type. Something the simple target filter cannot do. What do you think ? +1 I don't think this should block the framework patches that are on list now, though. I'll file a RFE for tuning how the default and type permissions look. Would that be fine? -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [RFE] Permissions V2
On 12/06/2013 03:49 PM, Simo Sorce wrote: On Fri, 2013-12-06 at 15:46 +0100, Petr Viktorin wrote: On 12/06/2013 03:28 PM, Simo Sorce wrote: On Fri, 2013-12-06 at 14:14 +0100, Petr Viktorin wrote: On 12/02/2013 02:48 PM, Petr Viktorin wrote: On 12/02/2013 02:29 PM, Simo Sorce wrote: It would be very nice if you can add the resulting LDAP objects in the example, that will allow me to reason on the correctness of the translation. OK, I'll work on that. I've added the resulting LDAP objects to the tests here: http://www.freeipa.org/index.php?title=V3/Permissions_V2/tests Thank you Petr, I was looking at them and I see we often use target=ldap://dn type for selecting which objects this apply to. This was sort of necessary when the permissions were all in the base and we wanted to limit to specific entries in subtrees. However I was wondering if we shouldn't transition/allow to user targetfilter or targetattrfilter (this would be needed to have add/delete permissions). For example, instead of: (target = ldap:///uid=*,cn=users,cn=accounts,$SUFFIX;) We could have: (targetfilter = (objectclass=ipaUser)) It also occurs to me we could do very neat things like allowing manager access with (targetfilter = (managedby=dn)), and similar. In general using targetfilter and targetattrfilter is more flexible and allow for applying different permission depending exacly on the object type or even specific sets of objects of a common type. Something the simple target filter cannot do. What do you think ? +1 I don't think this should block the framework patches that are on list now, though. I'll file a RFE for tuning how the default and type permissions look. Would that be fine? Do we need a new attribute, or do you think we can do this without changing the schema ? Ah, yes. Please reserve ipaPermTargetAttrFilter. (ipaPermTargetFilter is already reserved) -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [RFE] Permissions V2
On 12/06/2013 03:54 PM, Rob Crittenden wrote: Simo Sorce wrote: On Fri, 2013-12-06 at 15:46 +0100, Petr Viktorin wrote: On 12/06/2013 03:28 PM, Simo Sorce wrote: On Fri, 2013-12-06 at 14:14 +0100, Petr Viktorin wrote: On 12/02/2013 02:48 PM, Petr Viktorin wrote: On 12/02/2013 02:29 PM, Simo Sorce wrote: It would be very nice if you can add the resulting LDAP objects in the example, that will allow me to reason on the correctness of the translation. OK, I'll work on that. I've added the resulting LDAP objects to the tests here: http://www.freeipa.org/index.php?title=V3/Permissions_V2/tests Thank you Petr, I was looking at them and I see we often use target=ldap://dn type for selecting which objects this apply to. This was sort of necessary when the permissions were all in the base and we wanted to limit to specific entries in subtrees. However I was wondering if we shouldn't transition/allow to user targetfilter or targetattrfilter (this would be needed to have add/delete permissions). For example, instead of: (target = ldap:///uid=*,cn=users,cn=accounts,$SUFFIX;) We could have: (targetfilter = (objectclass=ipaUser)) It also occurs to me we could do very neat things like allowing manager access with (targetfilter = (managedby=dn)), and similar. In general using targetfilter and targetattrfilter is more flexible and allow for applying different permission depending exacly on the object type or even specific sets of objects of a common type. Something the simple target filter cannot do. What do you think ? +1 I don't think this should block the framework patches that are on list now, though. I'll file a RFE for tuning how the default and type permissions look. Would that be fine? Do we need a new attribute, or do you think we can do this without changing the schema ? Right now type implies the target used. I think it would just change to be a targetfilter instead (or a piece of one anyway). What may be tricky is combining a type and a user-profiled filter, and then decomposing that, without a separate attribute. The design changes --type to be a shortcut for setting location and filter at the same time. Those two are what's actually stored in LDAP, and on output type is added if those two match a known object type. We may (in the RFE I'm about to file) want to make targetfilter targetattrfilter multivalued, and e.g. make --type work on objectclass filters only. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 0333 test_webui: Allow False values in configuration for no_ca, no_dns, has_trusts
Hello,
As I'm consolidating test job configuration, I learned that the line
no_dns: False
in WebUI test config has the same effect as `no_dns: True`. This is
confusing, and it complicates generating the config.
Patch is untested because I don't have the WebUI test environment set up
locally. Petr, could you check if it works?
--
Petr³
From ff7581fd58701d3effb02dd5cc18f1f43491f17b Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Fri, 6 Dec 2013 16:39:06 +0100
Subject: [PATCH] test_webui: Allow False values in configuration for no_ca,
no_dns, has_trusts
The driver only checked if the corresponding value was in the config, so
no_dns: False
had the same effect as
no_dns: True
Change the check to take the value into consideration.
This makes false-y values like False (from YAML) and empty string
(from environment) work as if the value was not specified.
---
ipatests/test_webui/ui_driver.py | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/ipatests/test_webui/ui_driver.py b/ipatests/test_webui/ui_driver.py
index 1c396b6d389f2672b1d36f8c71361a3c84afcce7..cf95a8cdf6a9012cf194487b7cb5eaf7e4676f92 100644
--- a/ipatests/test_webui/ui_driver.py
+++ b/ipatests/test_webui/ui_driver.py
@@ -241,19 +241,19 @@ def has_ca(self):
FreeIPA server was installed with CA.
-return 'no_ca' not in self.config
+return not self.config.get('no_ca')
def has_dns(self):
FreeIPA server was installed with DNS.
-return 'no_dns' not in self.config
+return not self.config.get('no_dns')
def has_trusts(self):
FreeIPA server was installed with Trusts.
-return 'has_trusts' in self.config
+return self.config.get('has_trusts')
def has_active_request(self):
--
1.8.3.1
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 531 Fix license in some Web UI files
On 12/06/2013 05:10 PM, Simo Sorce wrote: On Fri, 2013-12-06 at 14:19 +0100, Petr Vobornik wrote: Modified web ui files had incorrect GPLv2 headers instead of GPLv3 ones. All of the affected code is of FreeIPA origin. Ack. Simo. Pushed to master: b6540e88d88470f6566507e442f521214c5a74dc ipa-3-3: 2877f5d8a11ebdd32c2007b26facab2073cf48ad -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 439 Allow kernel keyring CCACHE when supported
On 12/06/2013 03:00 PM, Simo Sorce wrote: On Fri, 2013-12-06 at 13:42 +0100, Martin Kosek wrote: On 12/02/2013 05:20 PM, Alexander Bokovoy wrote: On Mon, 02 Dec 2013, Martin Kosek wrote: On 12/02/2013 04:05 PM, Petr Viktorin wrote: On 12/02/2013 03:42 PM, Simo Sorce wrote: On Mon, 2013-12-02 at 14:51 +0100, Petr Viktorin wrote: On 12/02/2013 02:01 PM, Martin Kosek wrote: On 12/02/2013 01:58 PM, Petr Viktorin wrote: On 11/29/2013 01:48 PM, Martin Kosek wrote: On 11/19/2013 12:35 PM, Petr Viktorin wrote: On 11/05/2013 07:22 PM, Martin Kosek wrote: Server and client installer should allow kernel keyring ccache when supported. How do I enable the kernel keyring? On f20 I get this: 2013-11-19T11:28:07Z DEBUG Starting external process 2013-11-19T11:28:07Z DEBUG args=keyctl get_persistent @s 0 2013-11-19T11:28:07Z DEBUG Process finished, return code=1 2013-11-19T11:28:07Z DEBUG stdout= 2013-11-19T11:28:07Z DEBUG stderr=keyctl_get_persistent: Key has been revoked It should be enabled out of the box. But there were some initial issues with persistent keyring in the first versions of kernel with a support, hopefully this was just a fluke which disappeared. This is what I see on my F20 with kernel-3.11.9-300.fc20.x86_64: # keyctl get_persistent @s 0 637466038 With kernel-3.11.10-300.fc20.x86_64, I get an error again: $ keyctl get_persistent @s 0 keyctl_get_persistent: Key has been revoked Not sure if it is a typo, but you won't surely get a root's keyring as a non-root user... It is just a typo, but it looks like you got me on the right track. keyctl apparently needs a real root login: $ sudo keyctl get_persistent @s 0 keyctl_get_persistent: Key has been revoked $ sudo su # keyctl get_persistent @s 0 keyctl_get_persistent: Key has been revoked # exit $ sudo su - Last login: Mon Dec 2 14:09:36 CET 2013 on pts/1 # keyctl get_persistent @s 0 968622527 # logout Please use sudo -i to get an interactive 'login' shell. Unsurprisingly, when ipa-server-install is run from sudo, it complains that the key is unsupported. From a root login all is OK. Is that expected? You should run ipa-server-install using a login shell I think. Should we open a bug to detect this and fail ? It's always worked with just sudo for me. So yes, if it's required I think we should enforce it. Simo or Alexander, is there some way to find that out in a clean way? I mean if we are in an interactive login shell. Ideally, please also file a bug with this information :) Interactive or login? These two are different a bit. There is no general way because not all shells implement common approach to detect this. For example, echo $- | grep -q i would work in a Bourne-style shell for interactive shell shopt -q login_shell would give you a login shell detector in bash but test $options[LOGIN] = on would work for login shell in zsh, similarly INTERACTIVE index would give you state of interactive shell. I meant login shell - so that we do not have problems with checking the get_persistent keyctl command. I still do not fully understand the keyctl behavior, it is working on my kernel-3.11.9-300.fc20.x86_64 even with plain sudo: $ sudo keyctl get_persistent @s 0 I think the previous behavior was cause by the improper selinux handling in the kernel, and is fixed in the latest kernel. There is indeed no reason why get_persistent shouldn't work for non-login shell unless selinux policy explicitly disallows it for sudo like programs. Anyway, any opinions on this particular patch? I'd prefer to get it in soon and file enhancement ticket for the login terminal detection, if needed. I do not have any objections. Simo. ACK, pushed to * master: 9677308caa78ed722570aea32f21334b8c27bad3 * ipa-3-3: 5b2ce3c5a57e8193ee1c6d23c4e79c3b2b62cb05 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0330 - Add comment about last change to VERSION
On 12/09/2013 02:50 PM, Martin Kosek wrote: On 12/09/2013 02:35 PM, Simo Sorce wrote: On Mon, 2013-12-09 at 12:39 +0100, Martin Kosek wrote: On 12/09/2013 12:08 PM, Tomas Babej wrote: On 12/05/2013 01:37 PM, Petr Viktorin wrote: Consider this scenario: - Nathaniel submits RADIUS patches that update the API version (from 2.69 to 2.70) - I have ACI patches that also bump the version (from 2.69 to 2.70) - Nathaniel's patches gets accepted - I rebase my ACI patches onto master. Git thinks that the 2.69-2.70 change is already done, so it leaves VERSION unchanged. I can solve this locally by telling Git to not merge VERSION automatically, but I think it would be helpful to add a unique comment to each change so that everyone gets a conflict cases like this. Do you agree? ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Makes sense to me. I'd just add a comment so that the purpose of the last change comment is also obvious for the new developer perusing the VERSION file. Maybe something along the lines of: IPA_API_VERSION_MAJOR=2 IPA_API_VERSION_MINOR=70 +# Update the last change entry to enforce conflict on merging two independent branches into master. +# Last change: npmccallum - RADIUS support I don't think this is necessary, IMO Last change: is enough as instructions. I spoke with Petr offline, to me it would make bigger sense if we just forbid automatic merging of this line on the git server side (if possible) instead of adding other arbitrary work to our development process. IIRC, Petr3 said it should be possible to do. Except it may not fix the issue, if someone does a rebase on his machine and resubmit a patch to the list w/o noticing the change was effectively dropped. Simo. I thought that the point of the anti-merge protection is to prevent git merging tools to prevent automatic rebase of this particular line and force manual merging, i.e. force increasing the number. When the file is equal on both sides of the merge, Git just uses the common file, and doesn't consider it for merging. So unfortunately, git attributes won't work here; there needs to be another change in the file. I did say otherwise, sorry for that. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 441 Consolidate .gitignore entries
On 12/09/2013 05:43 PM, Martin Kosek wrote: Clean up the .gitignore file: - Remove no longer used .gitignore entries, like .bzr files - Do not repeat autotools generated files over and over again - Whitelist existent Makefiles in the repository - Better separate the .gitignore entries === When porting Jan's patches downstream, I had hard time merging changes to /Makefile in the repository as it was stated in .gitignore which made git (and me) suffer. I fixed that by whitelisting this one. Unfortunately, when I saw other entries in .gitignore I could not resist and refactored the file to make it (hopefully) simpler and easier to maintain. Martin Thanks! ACK, pushed to master: 1e0405880fb1855563cb9b58a39213e1d3b4a2c6 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
