Re: [Freeipa-devel] [PATCH] 917 user automember for ipa default user
On Mon, 2012-01-16 at 15:43 -0500, Rob Crittenden wrote:
Martin Kosek wrote:
On Mon, 2011-12-12 at 23:09 -0500, Rob Crittenden wrote:
Rob Crittenden wrote:
Rather than manually adding users to the default ipa users group
configure automember to do it for us.
This was quite simple for new installs but a bit complex on upgrades so
I implemented it as an update plugin.
I also added a unit test for the config module. The majority of config
is ignored for now. I'm afraid we'd run into too many false positives if
we test each element, and most of these just store data so there isn't a
lot that can go wrong.
rob
Small revision. I wasn't shipping the update plugin.
rob
I have few minor-ish issues:
0) I was thinking if this new approach for assignment of ipa default
users is safe enough. If user accidentally mess with automember and
modifies/deletes the default group rule, new users may be omitted from
the default group set in IPA config. Are we sure that we are OK with
this?
I made some stricter tests that don't allow users to manage the
conditions of the default users group nor use an existing rule with
conditions for the default users group.
1) Several tests are provided with a hard-code basedn
(dc=greyoak,dc=com). api.env.basedn would a better choice
Ouch, fixed.
2) We could optimize user.py not to retrieve config from LDAP since it
is now needed only when api.env.wait_for_attr is now. I think this may
speedup the command a little bit:
...
# Automember adds our user to the default group for us.
if self.api.env.wait_for_attr:
config = ldap.get_ipa_config()[1]
def_primary_group = config.get('ipadefaultprimarygroup')
newentry = wait_for_value(ldap, dn, 'memberOf',
def_primary_group)
entry_from_entry(entry_attrs, newentry)
...
Ok, that's a good idea. I think this path is going to go away soon
though once we have transactions in 389-ds.
rob
Thanks, it safer now. We just have to fix ipa-server-install too:
# ipa-server-install
...
[12/13]: restarting httpd
[13/13]: configuring httpd to start on boot
done configuring httpd.
Applying LDAP updates
Unexpected error - see ipaserver-install.log for details:
The default users group cannot be removed or modified
There is also a bug in is_default_users group - all non-group automember
rules are rejected:
# ipa hostgroup-add --desc=Web Servers webservers
Added hostgroup webservers
Host-group: webservers
Description: Web Servers
# ipa automember-add --type=hostgroup webservers
--
Added automember rule webservers
--
Automember Rule: webservers
# ipa automember-add-condition --key=fqdn --type=hostgroup
--inclusive-regex=^web[1-9]+\.example\.com webservers
ipa: ERROR: The default users group cannot be removed or modified
A buch of tests in test_automember_plugin.py is failing because of this
bug too.
Martin
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 916 make category and members mutually exclusive in Sudo
On Mon, 2012-01-16 at 22:20 -0500, Rob Crittenden wrote:
Rob Crittenden wrote:
Martin Kosek wrote:
On Mon, 2011-12-12 at 17:14 -0500, Rob Crittenden wrote:
This patch makes all categories and their equivalent members mutually
exclusive like in the HBAC plugin. So if you have usercat='all' you
can't add users.
Added test cases for these as well.
I also modified the default list of attributes to include the RunAs
attributes.
rob
NACK. I see several issues in this patch:
1) Error messages should be internationalized since they can be read by
a user (this problem is in HBAC too)
2) All constructs like this one can be simplified (and thus made less
error prone).
+ if 'cmdcategory' in _entry_attrs and \
+ _entry_attrs['cmdcategory'][0].lower() == 'all':
+ raise errors.MutuallyExclusiveError(reason=commands cannot be added
when command category='all')
can be changed to:
+ if _entry_attrs.get('cmdcategory', [''])[0].lower() == 'all':
+ raise errors.MutuallyExclusiveError(...)
I think the code would be then also more readable.
3) I think this code only works by an accident :-)
+ if ('ipasudorunasusercategory' or 'ipasudorunasgroupcategory') \
+ in _entry_attrs and \
+ (_entry_attrs['ipasudorunasusercategory'][0].lower() == 'all' or \
+ _entry_attrs['ipasudorunasgroupcategory'][0].lower() == 'all'):
+ raise errors.MutuallyExclusiveError(reason=users cannot be added
when runAs user or runAs group category='all')
('ipasudorunasusercategory' or 'ipasudorunasgroupcategory') simply
returns 'ipasudorunasusercategory'. Thus the check for
'ipasudorunasgroupcategory' in _entry_attrs is not performed at all.
Thanks to this bug, user is then able to pass a runAsGroup to sudorule
with groupcat == 'all':
# ipa sudorule-add foo --runasgroupcat=all
-
Added Sudo Rule foo
-
Rule name: foo
Enabled: TRUE
RunAs Group category: all
# ipa sudorule-add-runasuser foo --groups=admins
Rule name: foo
Enabled: TRUE
RunAs Group category: all
Groups of RunAs Users: admins
-
Number of members added 1
-
A change proposed in 1) could make the change simpler:
+ if _entry_attrs.get('ipasudorunasusercategory', [''])[0].lower() ==
'all' or \
+ _entry_attrs.get('ipasudorunasgroupcategory', [''])[0].lower() ==
'all':
+ raise ...
Martin
Updated patch attached. Using the is_all() function instead. Opened
separate ticket to internationalize HBAC exceptions,
https://fedorahosted.org/freeipa/ticket/2267
rob
Rebased against ipa-2-2.
rob
There are still few issues:
1) test_sudorule_plugin.py is missing in the new commit
2) The patch does not work for ipasudorunasusercategory or
ipasudorunasgroupcategory as they are not in self.obj.default_attributes
3) I also saw some internal errors:
# ipa sudorule-show foo --all
dn:
ipauniqueid=a0e84cda-40e5-11e1-a35b-00163e7228ea,cn=sudorules,cn=sudo,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com
Rule name: foo
Enabled: TRUE
User category: all
RunAs Group category: all
Groups of RunAs Users: admins
ipauniqueid: a0e84cda-40e5-11e1-a35b-00163e7228ea
objectclass: ipaassociation, ipasudorul
# ipa sudorule-add-user foo --users=admin
ipa: ERROR: an internal error has occurred
# ipa sudorule-add-user foo --groups=admins
ipa: ERROR: an internal error has occurred
# ipa sudorule-mod foo --hostcat=all
# ipa sudorule-add-host foo --hosts=`hostname`
ipa: ERROR: an internal error has occurred
Martin
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 192 Replace float with Decimal
On Fri, 2012-01-13 at 21:02 +0100, Martin Kosek wrote:
This patch fixes RHEL 6.2 build issue.
Having float type as a base type for floating point parameter in
ipalib introduces several issues, e.g. problem with representation
or value comparison. Python language provides Decimal type which
help overcome these issue.
This patch replaces a float type with Decimal type in Float
parameter. A precision attribute was added to Float parameter that
can be used to limit a number of decimal places in parameter
representation. This approach fixes a problem with API.txt
validation where comparison of float values may fail on different
architectures due to float representation error.
In order to safely transfer the parameter value over RPC it is
being converted to string which is then converted back to Decimal
number on server side.
https://fedorahosted.org/freeipa/ticket/2260
Sending an improved version of the patch with following major changes:
1) Float parameter was renamed to Decimal as it base type is different
and would confuse users otherwise.
2) Parameter maxvalue, minvalue and default can be also passed as a
string and not just as a decimal.Decimal value. Parameter definition is
then much simpler.
3) LDAP backend encoder was enhanced to support this new type (it
converts it to string just like a float value).
Martin
From 225398131666a18d76dffa87ae322c29b8106677 Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Tue, 17 Jan 2012 11:19:00 +0100
Subject: [PATCH] Replace float with Decimal
Having float type as a base type for floating point parameters in
ipalib introduces several issues, e.g. problem with representation
or value comparison. Python language provides a Decimal type which
help overcome these issues.
This patch replaces a float type and Float parameter with a
decimal.Decimal type in Decimal parameter. A precision attribute
was added to Decimal parameter that can be used to limit a number
of decimal places in parameter representation. This approach fixes
a problem with API.txt validation where comparison of float values
may fail on different architectures due to float representation error.
In order to safely transfer the parameter value over RPC it is
being converted to string which is then converted back to
decimal.Decimal number on a server side.
https://fedorahosted.org/freeipa/ticket/2260
---
API.txt | 36 +++---
doc/guide/guide.org |8 ++--
ipalib/__init__.py |2 +-
ipalib/encoder.py|6 ++-
ipalib/parameters.py | 85 +++---
ipalib/plugins/dns.py| 51
ipalib/rpc.py|4 ++
make-lint|2 +-
tests/test_ipalib/test_parameters.py | 47 ++-
tests/test_xmlrpc/test_dns_plugin.py |2 +-
10 files changed, 156 insertions(+), 87 deletions(-)
diff --git a/API.txt b/API.txt
index 9048231bb1f349047f9790e5335778d4c3d637b0..2937c24f4d6aa53b1028f430a05ef6453544ea7c 100644
--- a/API.txt
+++ b/API.txt
@@ -654,16 +654,16 @@ option: Str('kx_part_exchanger', attribute=False, cli_name='kx_exchanger', multi
option: LOCRecord('locrecord', attribute=True, cli_name='loc_rec', csv=True, multivalue=True, option_group=u'LOC Record', required=False)
option: Int('loc_part_lat_deg', attribute=False, cli_name='loc_lat_deg', maxvalue=90, minvalue=0, multivalue=False, option_group=u'LOC Record', required=False)
option: Int('loc_part_lat_min', attribute=False, cli_name='loc_lat_min', maxvalue=59, minvalue=0, multivalue=False, option_group=u'LOC Record', required=False)
-option: Float('loc_part_lat_sec', attribute=False, cli_name='loc_lat_sec', maxvalue=59.999, minvalue=0.0, multivalue=False, option_group=u'LOC Record', required=False)
+option: Decimal('loc_part_lat_sec', attribute=False, cli_name='loc_lat_sec', maxvalue=Decimal('59.999'), minvalue=Decimal('0.0'), multivalue=False, option_group=u'LOC Record', precision=3, required=False)
option: StrEnum('loc_part_lat_dir', attribute=False, cli_name='loc_lat_dir', multivalue=False, option_group=u'LOC Record', required=False, values=(u'N', u'S'))
option: Int('loc_part_lon_deg', attribute=False, cli_name='loc_lon_deg', maxvalue=180, minvalue=0, multivalue=False, option_group=u'LOC Record', required=False)
option: Int('loc_part_lon_min', attribute=False, cli_name='loc_lon_min', maxvalue=59, minvalue=0, multivalue=False, option_group=u'LOC Record', required=False)
-option: Float('loc_part_lon_sec', attribute=False, cli_name='loc_lon_sec', maxvalue=59.999, minvalue=0.0, multivalue=False, option_group=u'LOC Record', required=False)
+option: Decimal('loc_part_lon_sec', attribute=False, cli_name='loc_lon_sec', maxvalue=Decimal('59.999'), minvalue=Decimal('0.0'), multivalue=False, option_group=u'LOC Record', precision=3, required=False)
option:
Re: [Freeipa-devel] [PATCH] 192 Replace float with Decimal
On Tue, 2012-01-17 at 11:27 +0100, Martin Kosek wrote:
On Fri, 2012-01-13 at 21:02 +0100, Martin Kosek wrote:
This patch fixes RHEL 6.2 build issue.
Having float type as a base type for floating point parameter in
ipalib introduces several issues, e.g. problem with representation
or value comparison. Python language provides Decimal type which
help overcome these issue.
This patch replaces a float type with Decimal type in Float
parameter. A precision attribute was added to Float parameter that
can be used to limit a number of decimal places in parameter
representation. This approach fixes a problem with API.txt
validation where comparison of float values may fail on different
architectures due to float representation error.
In order to safely transfer the parameter value over RPC it is
being converted to string which is then converted back to Decimal
number on server side.
https://fedorahosted.org/freeipa/ticket/2260
Sending an improved version of the patch with following major changes:
1) Float parameter was renamed to Decimal as it base type is different
and would confuse users otherwise.
2) Parameter maxvalue, minvalue and default can be also passed as a
string and not just as a decimal.Decimal value. Parameter definition is
then much simpler.
3) LDAP backend encoder was enhanced to support this new type (it
converts it to string just like a float value).
Martin
I forgot to add an encoding rule to JSON xmlrpc server. This can be
useful in a future when Decimal type is actually used for a real LDAP
attribute.
Martin
From 7c3ebfa08df24be75644b0c66a109930a80c27e8 Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Tue, 17 Jan 2012 11:19:00 +0100
Subject: [PATCH] Replace float with Decimal
Having float type as a base type for floating point parameters in
ipalib introduces several issues, e.g. problem with representation
or value comparison. Python language provides a Decimal type which
help overcome these issues.
This patch replaces a float type and Float parameter with a
decimal.Decimal type in Decimal parameter. A precision attribute
was added to Decimal parameter that can be used to limit a number
of decimal places in parameter representation. This approach fixes
a problem with API.txt validation where comparison of float values
may fail on different architectures due to float representation error.
In order to safely transfer the parameter value over RPC it is
being converted to string which is then converted back to
decimal.Decimal number on a server side.
https://fedorahosted.org/freeipa/ticket/2260
---
API.txt | 36 +++---
doc/guide/guide.org |8 ++--
ipalib/__init__.py |2 +-
ipalib/encoder.py|6 ++-
ipalib/parameters.py | 85 +++---
ipalib/plugins/dns.py| 51
ipalib/rpc.py|4 ++
ipaserver/rpcserver.py |3 +
make-lint|2 +-
tests/test_ipalib/test_parameters.py | 47 ++-
tests/test_xmlrpc/test_dns_plugin.py |2 +-
11 files changed, 159 insertions(+), 87 deletions(-)
diff --git a/API.txt b/API.txt
index 9048231bb1f349047f9790e5335778d4c3d637b0..2937c24f4d6aa53b1028f430a05ef6453544ea7c 100644
--- a/API.txt
+++ b/API.txt
@@ -654,16 +654,16 @@ option: Str('kx_part_exchanger', attribute=False, cli_name='kx_exchanger', multi
option: LOCRecord('locrecord', attribute=True, cli_name='loc_rec', csv=True, multivalue=True, option_group=u'LOC Record', required=False)
option: Int('loc_part_lat_deg', attribute=False, cli_name='loc_lat_deg', maxvalue=90, minvalue=0, multivalue=False, option_group=u'LOC Record', required=False)
option: Int('loc_part_lat_min', attribute=False, cli_name='loc_lat_min', maxvalue=59, minvalue=0, multivalue=False, option_group=u'LOC Record', required=False)
-option: Float('loc_part_lat_sec', attribute=False, cli_name='loc_lat_sec', maxvalue=59.999, minvalue=0.0, multivalue=False, option_group=u'LOC Record', required=False)
+option: Decimal('loc_part_lat_sec', attribute=False, cli_name='loc_lat_sec', maxvalue=Decimal('59.999'), minvalue=Decimal('0.0'), multivalue=False, option_group=u'LOC Record', precision=3, required=False)
option: StrEnum('loc_part_lat_dir', attribute=False, cli_name='loc_lat_dir', multivalue=False, option_group=u'LOC Record', required=False, values=(u'N', u'S'))
option: Int('loc_part_lon_deg', attribute=False, cli_name='loc_lon_deg', maxvalue=180, minvalue=0, multivalue=False, option_group=u'LOC Record', required=False)
option: Int('loc_part_lon_min', attribute=False, cli_name='loc_lon_min', maxvalue=59, minvalue=0, multivalue=False, option_group=u'LOC Record', required=False)
-option: Float('loc_part_lon_sec', attribute=False, cli_name='loc_lon_sec', maxvalue=59.999, minvalue=0.0,
Re: [Freeipa-devel] [PATCH] 916 make category and members mutually exclusive in Sudo
Martin Kosek wrote:
On Mon, 2012-01-16 at 22:20 -0500, Rob Crittenden wrote:
Rob Crittenden wrote:
Martin Kosek wrote:
On Mon, 2011-12-12 at 17:14 -0500, Rob Crittenden wrote:
This patch makes all categories and their equivalent members mutually
exclusive like in the HBAC plugin. So if you have usercat='all' you
can't add users.
Added test cases for these as well.
I also modified the default list of attributes to include the RunAs
attributes.
rob
NACK. I see several issues in this patch:
1) Error messages should be internationalized since they can be read by
a user (this problem is in HBAC too)
2) All constructs like this one can be simplified (and thus made less
error prone).
+ if 'cmdcategory' in _entry_attrs and \
+ _entry_attrs['cmdcategory'][0].lower() == 'all':
+ raise errors.MutuallyExclusiveError(reason=commands cannot be added
when command category='all')
can be changed to:
+ if _entry_attrs.get('cmdcategory', [''])[0].lower() == 'all':
+ raise errors.MutuallyExclusiveError(...)
I think the code would be then also more readable.
3) I think this code only works by an accident :-)
+ if ('ipasudorunasusercategory' or 'ipasudorunasgroupcategory') \
+ in _entry_attrs and \
+ (_entry_attrs['ipasudorunasusercategory'][0].lower() == 'all' or \
+ _entry_attrs['ipasudorunasgroupcategory'][0].lower() == 'all'):
+ raise errors.MutuallyExclusiveError(reason=users cannot be added
when runAs user or runAs group category='all')
('ipasudorunasusercategory' or 'ipasudorunasgroupcategory') simply
returns 'ipasudorunasusercategory'. Thus the check for
'ipasudorunasgroupcategory' in _entry_attrs is not performed at all.
Thanks to this bug, user is then able to pass a runAsGroup to sudorule
with groupcat == 'all':
# ipa sudorule-add foo --runasgroupcat=all
-
Added Sudo Rule foo
-
Rule name: foo
Enabled: TRUE
RunAs Group category: all
# ipa sudorule-add-runasuser foo --groups=admins
Rule name: foo
Enabled: TRUE
RunAs Group category: all
Groups of RunAs Users: admins
-
Number of members added 1
-
A change proposed in 1) could make the change simpler:
+ if _entry_attrs.get('ipasudorunasusercategory', [''])[0].lower() ==
'all' or \
+ _entry_attrs.get('ipasudorunasgroupcategory', [''])[0].lower() ==
'all':
+ raise ...
Martin
Updated patch attached. Using the is_all() function instead. Opened
separate ticket to internationalize HBAC exceptions,
https://fedorahosted.org/freeipa/ticket/2267
rob
Rebased against ipa-2-2.
rob
There are still few issues:
1) test_sudorule_plugin.py is missing in the new commit
2) The patch does not work for ipasudorunasusercategory or
ipasudorunasgroupcategory as they are not in self.obj.default_attributes
3) I also saw some internal errors:
# ipa sudorule-show foo --all
dn:
ipauniqueid=a0e84cda-40e5-11e1-a35b-00163e7228ea,cn=sudorules,cn=sudo,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com
Rule name: foo
Enabled: TRUE
User category: all
RunAs Group category: all
Groups of RunAs Users: admins
ipauniqueid: a0e84cda-40e5-11e1-a35b-00163e7228ea
objectclass: ipaassociation, ipasudorul
# ipa sudorule-add-user foo --users=admin
ipa: ERROR: an internal error has occurred
# ipa sudorule-add-user foo --groups=admins
ipa: ERROR: an internal error has occurred
# ipa sudorule-mod foo --hostcat=all
# ipa sudorule-add-host foo --hosts=`hostname`
ipa: ERROR: an internal error has occurred
Martin
Somehow sent the wrong version of the patch. This one should be better.
rob
From 9426423635d3cb527216859370b5b1cbdd77cc32 Mon Sep 17 00:00:00 2001
From: Rob Crittenden rcrit...@redhat.com
Date: Fri, 13 Jan 2012 11:34:04 -0500
Subject: [PATCH] In sudo when the category is all do not allow members, and
vice versa.
This is what we already do in the HBAC plugin, this ports it to Sudo.
If a category (user, host, etc) is u'all' then we don't allow individual
members be added. Conversely if there are members we don't allow the
category be set to u'all'.
https://fedorahosted.org/freeipa/ticket/1440
---
ipalib/plugins/hbacrule.py| 11 ++-
ipalib/plugins/sudorule.py| 75 ++
tests/test_xmlrpc/test_sudorule_plugin.py | 98 -
3 files changed, 177 insertions(+), 7 deletions(-)
diff --git a/ipalib/plugins/hbacrule.py b/ipalib/plugins/hbacrule.py
index 92b656d66f971b0d2b6fa1d4f1ff8413b45cc819..0fa44a5903cf35715d0d97acb5aa0a5eb58ddf76 100644
--- a/ipalib/plugins/hbacrule.py
+++ b/ipalib/plugins/hbacrule.py
@@ -96,10 +96,13 @@ def is_all(options, attribute):
See if options[attribute] is lower-case 'all' in a safe way.
-if attribute in options and \
-options[attribute] is not None and \
-options[attribute].lower() == 'all':
-return True
+if attribute in options and options[attribute] is not None:
+if type(options[attribute]) in (list, tuple):
+
[Freeipa-devel] [PATCH] 926 fix s4u2proxy update file
I used the wrong template variable for hosts. rob From f8f196cfaf04f41322ef7337d68d343fff579fd1 Mon Sep 17 00:00:00 2001 From: Rob Crittenden rcrit...@redhat.com Date: Tue, 17 Jan 2012 09:09:15 -0500 Subject: [PATCH] Use correct template variable for hosts, FQDN. https://fedorahosted.org/freeipa/ticket/2268 --- install/updates/30-s4u2proxy.update |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/install/updates/30-s4u2proxy.update b/install/updates/30-s4u2proxy.update index be1d557..0775a69 100644 --- a/install/updates/30-s4u2proxy.update +++ b/install/updates/30-s4u2proxy.update @@ -8,11 +8,11 @@ default: objectClass: ipaKrb5DelegationACL default: objectClass: groupOfPrincipals default: objectClass: top default: cn: ipa-http-delegation -default: memberPrincipal: HTTP/$HOST@$REALM +default: memberPrincipal: HTTP/$FQDN@$REALM default: ipaAllowedTarget: 'cn=ipa-ldap-delegation-targets,cn=etc,$SUFFIX' dn: cn=ipa-ldap-delegation-targets,cn=s4u2proxy,cn=etc,$SUFFIX default: objectClass: groupOfPrincipals default: objectClass: top default: cn: ipa-ldap-delegation-targets -default: memberPrincipal: ldap/$HOST@$REALM +default: memberPrincipal: ldap/$FQDN@$REALM -- 1.7.6 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 917 user automember for ipa default user
Martin Kosek wrote:
On Mon, 2012-01-16 at 15:43 -0500, Rob Crittenden wrote:
Martin Kosek wrote:
On Mon, 2011-12-12 at 23:09 -0500, Rob Crittenden wrote:
Rob Crittenden wrote:
Rather than manually adding users to the default ipa users group
configure automember to do it for us.
This was quite simple for new installs but a bit complex on upgrades so
I implemented it as an update plugin.
I also added a unit test for the config module. The majority of config
is ignored for now. I'm afraid we'd run into too many false positives if
we test each element, and most of these just store data so there isn't a
lot that can go wrong.
rob
Small revision. I wasn't shipping the update plugin.
rob
I have few minor-ish issues:
0) I was thinking if this new approach for assignment of ipa default
users is safe enough. If user accidentally mess with automember and
modifies/deletes the default group rule, new users may be omitted from
the default group set in IPA config. Are we sure that we are OK with
this?
I made some stricter tests that don't allow users to manage the
conditions of the default users group nor use an existing rule with
conditions for the default users group.
1) Several tests are provided with a hard-code basedn
(dc=greyoak,dc=com). api.env.basedn would a better choice
Ouch, fixed.
2) We could optimize user.py not to retrieve config from LDAP since it
is now needed only when api.env.wait_for_attr is now. I think this may
speedup the command a little bit:
...
# Automember adds our user to the default group for us.
if self.api.env.wait_for_attr:
config = ldap.get_ipa_config()[1]
def_primary_group = config.get('ipadefaultprimarygroup')
newentry = wait_for_value(ldap, dn, 'memberOf',
def_primary_group)
entry_from_entry(entry_attrs, newentry)
...
Ok, that's a good idea. I think this path is going to go away soon
though once we have transactions in 389-ds.
rob
Thanks, it safer now. We just have to fix ipa-server-install too:
# ipa-server-install
...
[12/13]: restarting httpd
[13/13]: configuring httpd to start on boot
done configuring httpd.
Applying LDAP updates
Unexpected error - see ipaserver-install.log for details:
The default users group cannot be removed or modified
There is also a bug in is_default_users group - all non-group automember
rules are rejected:
# ipa hostgroup-add --desc=Web Servers webservers
Added hostgroup webservers
Host-group: webservers
Description: Web Servers
# ipa automember-add --type=hostgroup webservers
--
Added automember rule webservers
--
Automember Rule: webservers
# ipa automember-add-condition --key=fqdn --type=hostgroup
--inclusive-regex=^web[1-9]+\.example\.com webservers
ipa: ERROR: The default users group cannot be removed or modified
A buch of tests in test_automember_plugin.py is failing because of this
bug too.
Martin
Ah, I was just running the config tests :-(
The is_default_users_group() was trivial and fixed all but two tests. It
did however show a potentially fatal problem to the patch.
If we use automember for users then the default group will NEVER get
used because we guarantee that users are always added to one automember
group (ipausers). This sort of defeats the purpose of being able to set
a default group. So I'm thinking we'll need to drop this patch.
rob
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 924 display both hex and decimal serial numbers
Dne 16.1.2012 22:02, Rob Crittenden napsal(a): Rob Crittenden wrote: Jan Cholasta wrote: Dne 13.1.2012 20:53, Rob Crittenden napsal(a): When viewing a certificate it will show the serial number as hex (dec). # ipa service-show HTTP/rawhide.example.com Principal: HTTP/rawhide.example@example.com Certificate: [snip] Keytab: True Managed by: rawhide.example.com Subject: CN=rawhide.example.com,O=EXAMPLE.COM Serial Number: 0x403 (1027) Issuer: CN=EXAMPLE.COM Certificate Authority Not Before: Fri Jan 13 15:00:44 2012 UTC Not After: Thu Jan 13 15:00:44 2022 UTC Fingerprint (MD5): e5:43:17:0d:8d:af:d6:69:d8:fb:eb:ca:79:fb:47:69 Fingerprint (SHA1): c2:9e:8e:de:42:c9:4a:29:cc:b0:a0:de:57:c7:b7:d8:f9:b5:fe:e6 rob NACK Displaying a host or a service in the webUI fails with IPA error 3009: invalid 'serial_number': Decimal or hexadecimal number is required for serial number. I would suggest to do the nifty formatting of serial numbers on the client side, that would fix the webUI issue, allow non-IPA clients to parse the number without dissecting the string representation of it and probably also save me a hack in the type conversion overhaul. You could for example add a parameter flag like format_serial_number to indicate to the client that it should format the value as a serial number. Honza Well, we want to do as little client formatting as possible. The idea is to have a very thin client. It doesn't seem right to me to enforce this specific representation of what is really just an integer at the API level. Doing a little formatting on the client side won't make the client(s) particularly fat, will it? IMHO there is too much stuff done on server that would make more sense to do on client anyway (especially CLI-specific stuff such as CSV parsing). What is the reason we want such a thin client? I believe there should be clear separation of presentation and content, but perhaps I'm a little bit too idealistic :-). I'll look into fixing the UI side. I don't see this error in services, it displays correctly. I'm not sure if it is my browser or what but hosts don't display much of anything for me. rob I have just checked both master and ipa-2-2 and I'm getting the same error message (tested in Firefox 9.0.1) when viewing details of a host or a service with the usercertificate attribute set. BTW, wouldn't it make sense to format serial numbers in the cert plugin in the same way? Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 068 UI for SELinux user mapping
On 1/13/2012 8:58 AM, Petr Vobornik wrote: This patch adds UI for SELinux user mapping. Its design is based on HBAC Rule design. https://fedorahosted.org/freeipa/ticket/2145 Probably some labels and menu position should be changed. Live demo with limited functionality is at http://pvoborni.fedorapeople.org/selinuxusermap/#policy=selinuxusermapnavigation=policy Note: the patch is zipped because it has 760KB (update of metadata .json files) One issue, the selinux.js is not installed. It needs to be included in Makefile.am. Other than that it's ACKed, feel free to push after fixing it. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 068 UI for SELinux user mapping
On 1/13/2012 8:58 AM, Petr Vobornik wrote: This patch adds UI for SELinux user mapping. Its design is based on HBAC Rule design. https://fedorahosted.org/freeipa/ticket/2145 Probably some labels and menu position should be changed. Live demo with limited functionality is at http://pvoborni.fedorapeople.org/selinuxusermap/#policy=selinuxusermapnavigation=policy Note: the patch is zipped because it has 760KB (update of metadata .json files) One issue, the selinux.js is not installed. It needs to be included in Makefile.am. Other than that it's ACKed, feel free to push after fixing it. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] [WIP] 069 Added refresh button for UI
On 1/16/2012 8:02 AM, Petr Vobornik wrote: 1) Button position: I added the button into facet header next to 'add', 'delete', 'reset', 'update' buttons as shown on the picture ( http://pvoborni.fedorapeople.org/images/2051-refresh-button.png ). I'm not sure if it's the right position. I can also imagine it somewhere in the page header - in the green area on the right on the top main menu (Identity, Policy...) level. I think it's the right place, but UXD might have some idea about the button order. 2) Button icon: I used reset button icon. This is not good because in details facet reset and refresh button have the same icon. I think this icon is more suitable for refresh button and reset button should get new icon (maybe with reverse direction of the arrow). Probably we should ask UXD for opinion and icon. Yeah, maybe the Reset button probably should look like an Undo button: http://www.iconarchive.com/show/must-have-icons-by-visualpharm/Undo-icon.html The code is ACKed, feel free to push after feedback from UXD. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 926 fix s4u2proxy update file
On Tue, 2012-01-17 at 09:13 -0500, Rob Crittenden wrote: I used the wrong template variable for hosts. rob ACK. Works fine now. Pushed to master, ipa-2-2. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 61] Cache authentication in session
John Dennis wrote: Both of these are defined in ipalib/rpc.py (among others): +KRB5_CC_NOTFOUND = -1765328243 # Matching credential not found +KRB5_FCC_NOFILE = -1765328189 # No credentials cache found Perhaps all those defines should be moved to krb_utils.py. RPM build errors on non-SysV systems: File listed twice: /usr/share/ipa/ui/extension.js Installed (but unpackaged) file(s) found: /etc/rc.d/init.d/ipa_memcached make: *** [rpms] Error 1 (extention.js isn't yours) In the ipa_memcached service PID_PATH needs to be PIDFILE. It would be nice if sessions worked with the lite-server. I am unable to view the web UI. It just loops requesting all the the javascript files over and over again. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 927 fix deleting hbac rules when selinux user maps are involved
When deleting an HBAC rule we need to ensure that an SELinux user map
isn't pointing at it. The search for this didn't work well at all.
This patch corrects the search and makes it more specific.
I also tested that it works with the --continue flag of hbacrule-del.
The ticket has instructions on testing.
rob
From 456ff71bbf4dd2df2a5f06c424c1276dce09fde7 Mon Sep 17 00:00:00 2001
From: Rob Crittenden rcrit...@redhat.com
Date: Tue, 17 Jan 2012 17:54:00 -0500
Subject: [PATCH] Fix deletion of HBAC Rules when there are SELinux user maps
defined
When deleting an HBAC rule we need to ensure that an SELinux user
map isn't pointing at it. We need to take what is the cn of the HBAC
rule and see if that rule exists, then return the dn to that rule.
The search was not being done properly and wasn't enforcing uniqueness.
It could have returned partial matches as well (so tests for the
search test).
https://fedorahosted.org/freeipa/ticket/2269
---
ipalib/plugins/hbacrule.py |2 +-
ipalib/plugins/selinuxusermap.py| 14 +++--
tests/test_xmlrpc/test_selinuxusermap_plugin.py | 35 +++
3 files changed, 47 insertions(+), 4 deletions(-)
diff --git a/ipalib/plugins/hbacrule.py b/ipalib/plugins/hbacrule.py
index 0fa44a5903cf35715d0d97acb5aa0a5eb58ddf76..53d25aac697c78d78428c023efba6780d3d46633 100644
--- a/ipalib/plugins/hbacrule.py
+++ b/ipalib/plugins/hbacrule.py
@@ -243,7 +243,7 @@ class hbacrule_del(LDAPDelete):
msg_summary = _('Deleted HBAC rule %(value)s')
def pre_callback(self, ldap, dn, *keys, **options):
-kw = dict(seealso=dn)
+kw = dict(seealso=keys[0])
_entries = api.Command.selinuxusermap_find(None, **kw)
if _entries['count']:
raise errors.DependentEntry(key=keys[0], label=self.api.Object['selinuxusermap'].label_singular, dependent=_entries['result'][0]['cn'][0])
diff --git a/ipalib/plugins/selinuxusermap.py b/ipalib/plugins/selinuxusermap.py
index 475376f6efc9a8e17abe7b5553173b2b57142170..0b33b2b86252854a726515fa7183287a3ef3738c 100644
--- a/ipalib/plugins/selinuxusermap.py
+++ b/ipalib/plugins/selinuxusermap.py
@@ -299,11 +299,19 @@ class selinuxusermap_find(LDAPSearch):
def execute(self, *args, **options):
# If searching on hbacrule we need to find the uuid to search on
if 'seealso' in options:
-kw = dict(cn=options['seealso'], all=True)
+hbacrule = options['seealso']
+kw = dict(cn=hbacrule, all=True)
_entries = api.Command.hbacrule_find(None, **kw)['result']
del options['seealso']
-if _entries:
-options['seealso'] = _entries[0]['dn']
+found = False
+# look for an exact match. The search may return partial
+# matches.
+for entry in _entries:
+if entry['cn'][0] == hbacrule:
+found = True
+options['seealso'] = entry['dn']
+if not found:
+return dict(count=0, result=[], truncated=False)
return super(selinuxusermap_find, self).execute(*args, **options)
diff --git a/tests/test_xmlrpc/test_selinuxusermap_plugin.py b/tests/test_xmlrpc/test_selinuxusermap_plugin.py
index 368037dbef59695c9600f84489b79d7df12c8629..2fdccf3ef423f1af9d6e6f8ecb601e54aef9a2de 100644
--- a/tests/test_xmlrpc/test_selinuxusermap_plugin.py
+++ b/tests/test_xmlrpc/test_selinuxusermap_plugin.py
@@ -36,6 +36,7 @@ host1 = u'testhost1.%s' % api.env.domain
hostdn1 = DN(('fqdn',host1),('cn','computers'),('cn','accounts'),
api.env.basedn)
hbacrule1 = u'testhbacrule1'
+hbacrule2 = u'testhbacrule12'
fuzzy_selinuxusermapdn = Fuzzy(
'ipauniqueid=[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12},%s,%s' % (api.env.container_selinux, api.env.basedn)
@@ -51,6 +52,7 @@ class test_selinuxusermap(Declarative):
('user_del', [user1], {}),
('host_del', [host1], {}),
('hbacrule_del', [hbacrule1], {}),
+('hbacrule_del', [hbacrule2], {}),
]
tests = [
@@ -310,6 +312,26 @@ class test_selinuxusermap(Declarative):
),
+dict(
+desc='Create HBAC rule %r' % hbacrule2,
+command=(
+'hbacrule_add', [hbacrule2], {}
+),
+expected=dict(
+value=hbacrule2,
+summary=u'Added HBAC rule %s' % hbacrule2,
+result=dict(
+cn=[hbacrule2],
+objectclass=objectclasses.hbacrule,
+ipauniqueid=[fuzzy_uuid],
+accessruletype=[u'allow'],
+ipaenabledflag=[u'TRUE'],
+dn=fuzzy_hbacruledn,
+),
+),
+),
+
+
###
# Fill out rule with members and/or pointers to HBAC rules
dict(
@@ -542,6 +564,19 @@ class
Re: [Freeipa-devel] [PATCH] 924 display both hex and decimal serial numbers
Jan Cholasta wrote: Dne 16.1.2012 22:02, Rob Crittenden napsal(a): Rob Crittenden wrote: Jan Cholasta wrote: Dne 13.1.2012 20:53, Rob Crittenden napsal(a): When viewing a certificate it will show the serial number as hex (dec). # ipa service-show HTTP/rawhide.example.com Principal: HTTP/rawhide.example@example.com Certificate: [snip] Keytab: True Managed by: rawhide.example.com Subject: CN=rawhide.example.com,O=EXAMPLE.COM Serial Number: 0x403 (1027) Issuer: CN=EXAMPLE.COM Certificate Authority Not Before: Fri Jan 13 15:00:44 2012 UTC Not After: Thu Jan 13 15:00:44 2022 UTC Fingerprint (MD5): e5:43:17:0d:8d:af:d6:69:d8:fb:eb:ca:79:fb:47:69 Fingerprint (SHA1): c2:9e:8e:de:42:c9:4a:29:cc:b0:a0:de:57:c7:b7:d8:f9:b5:fe:e6 rob NACK Displaying a host or a service in the webUI fails with IPA error 3009: invalid 'serial_number': Decimal or hexadecimal number is required for serial number. I would suggest to do the nifty formatting of serial numbers on the client side, that would fix the webUI issue, allow non-IPA clients to parse the number without dissecting the string representation of it and probably also save me a hack in the type conversion overhaul. You could for example add a parameter flag like format_serial_number to indicate to the client that it should format the value as a serial number. Honza Well, we want to do as little client formatting as possible. The idea is to have a very thin client. It doesn't seem right to me to enforce this specific representation of what is really just an integer at the API level. Doing a little formatting on the client side won't make the client(s) particularly fat, will it? Yes. The current code just outputs labels and data. There is no if it is this attribute then do that logic. IMHO there is too much stuff done on server that would make more sense to do on client anyway (especially CLI-specific stuff such as CSV parsing). What is the reason we want such a thin client? To avoid double work such that every time we want a formatting change we have to change it in multiple places. This lesson was learned in v1. I believe there should be clear separation of presentation and content, but perhaps I'm a little bit too idealistic :-). You have a point, serial number is defined as an integer. Perhaps we should revisit this decision to display hex at all. I'll look into fixing the UI side. I don't see this error in services, it displays correctly. I'm not sure if it is my browser or what but hosts don't display much of anything for me. rob I have just checked both master and ipa-2-2 and I'm getting the same error message (tested in Firefox 9.0.1) when viewing details of a host or a service with the usercertificate attribute set. BTW, wouldn't it make sense to format serial numbers in the cert plugin in the same way? Perhaps. Like I said, I'm not really in favor of this change. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
