Re: [Freeipa-devel] [PATCHES 0024-0025] Improvements to idrange.py

2013-02-26 Thread Martin Kosek
On 02/25/2013 02:16 PM, Tomas Babej wrote:
 On Fri 22 Feb 2013 04:34:55 PM CET, Martin Kosek wrote:
 On 02/22/2013 03:01 PM, Tomas Babej wrote:
 On 02/21/2013 02:22 PM, Martin Kosek wrote:
 On 02/20/2013 03:19 PM, Tomas Babej wrote:
 On Wed 20 Feb 2013 02:24:03 PM CET, Alexander Bokovoy wrote:
 On Wed, 20 Feb 2013, Tomas Babej wrote:
 On 12/21/2012 12:15 PM, Tomas Babej wrote:
 Hi,

 Sending updated and rebased versions of patches 0024 and 0025.

 Tomas


 Sending rebased version, these got quite rotten.
 Thanks for updating them.

 @@ -504,25 +515,37 @@ class idrange_mod(LDAPUpdate):
   'not be found. Please specify the SID
 directly '
   'using dom-sid option.'))

 -try:
 -(old_dn, old_attrs) = ldap.get_entry(dn,
 -['ipabaseid',
 -'ipaidrangesize',
 -'ipabaserid',
 -'ipasecondarybaserid'])
 -except errors.NotFound:
 -self.obj.handle_not_found(*keys)
 +if in_updated_attrs('ipanttrusteddomainsid'):
 +if in_updated_attrs('ipasecondarybaserid'):
 +raise errors.ValidationError(name='ID Range setup',
 +error=_('Options dom_sid and secondary_rid_base
 cannot '
 +'be used together'))
 Since we agreed to refer to options by their CLI name (--dom-sid and
 --secondary-rid-base) in the other patch, it makes sense to use it
 here too.


 -if is_set('ipanttrusteddomainsid'):
 -# Validate SID as the one of trusted domains
 -
 self.obj.validate_trusted_domain_sid(entry_attrs['ipanttrusteddomainsid'])

 +if not in_updated_attrs('ipabaserid'):
 +raise errors.ValidationError(name='ID Range setup',
 +error=_('Options dom_sid and rid_base must '
 +'be used together'))
 Same here.

 +# secondary base rid must be set if and only if base rid
 is set
 +if in_updated_attrs('ipasecondarybaserid') !=\
 +in_updated_attrs('ipabaserid'):
 +raise errors.ValidationError(name='ID Range setup',
 +error=_('Options secondary_rid_base and rid_base
 must '
 +'be used together'))
 Same here.

 +dict(
 +desc='Try to modify ID range %r so it has only primary
 rid range set' % (testrange8),
 +command=('idrange_mod', [testrange8],
 +  dict(ipabaserid=testrange8_base_rid)),
 +expected=errors.ValidationError(
 +name='ID Range setup', error='Options
 secondary_rid_base and rid_base must be used together'),
 +),
 And synchronize error message here too.

 Thanks!

 Sending the updated patch 0024.

 Tomas

 In patch 0024 your intention is OK, but the checking functions are not:

is_set = lambda x: (x in entry_attrs) and (x is not None)
 +in_updated_attrs = lambda x: any((x in attrs and x is not None)
 + for attrs in (entry_attrs,
 old_attrs))

 They return True even when the attribute is None because they check if *x* 
 is
 None and not if *attrs[x]* is None. Example:

 # ipa idrange-add --base-id=120 --range-size=20 --rid-base=1000
 --secondary-rid-base=100 local_range
 
 Added ID range local_range
 
 Range name: local_range
 First Posix ID of the range: 120
 Number of IDs in the range: 20
 First RID of the corresponding RID range: 1000
 First RID of the secondary RID range: 100
 Range type: local domain range

 This command should be NOOP, but is not:

 # ipa idrange-mod local_range --dom-sid=
 ipa: ERROR: invalid 'ID Range setup': Options dom-sid and 
 secondary-rid-base
 cannot be used together

 Martin
 Thanks for the catch, all checking functions fixed.

 Sending the complete patchset, up to date.

 Tomas

 I think I am being a nitpicker now (sorry), but this condition also fails if
 the old_attrs has a setting, but I am removing it in a current -mod command:

 # ipa idrange-add --base-id=120 --range-size=20 --rid-base=1000
 --secondary-rid-base=100 local_range
 
 Added ID range local_range
 
Range name: local_range
First Posix ID of the range: 120
Number of IDs in the range: 20
First RID of the corresponding RID range: 1000
First RID of the secondary RID range: 100
Range type: local domain range
 # ipa idrange-mod local_range --dom-sid S-1-2 --secondary-rid-base=
 ipa: ERROR: invalid 'ID Range setup': Options dom-sid and secondary-rid-base
 cannot be used together

 Martin
 
 Yep. Ok, the command should now handle this use case as 

Re: [Freeipa-devel] [PATCHES 0024-0025] Improvements to idrange.py

2013-02-25 Thread Tomas Babej

On Fri 22 Feb 2013 04:34:55 PM CET, Martin Kosek wrote:

On 02/22/2013 03:01 PM, Tomas Babej wrote:

On 02/21/2013 02:22 PM, Martin Kosek wrote:

On 02/20/2013 03:19 PM, Tomas Babej wrote:

On Wed 20 Feb 2013 02:24:03 PM CET, Alexander Bokovoy wrote:

On Wed, 20 Feb 2013, Tomas Babej wrote:

On 12/21/2012 12:15 PM, Tomas Babej wrote:

Hi,

Sending updated and rebased versions of patches 0024 and 0025.

Tomas



Sending rebased version, these got quite rotten.

Thanks for updating them.


@@ -504,25 +515,37 @@ class idrange_mod(LDAPUpdate):
  'not be found. Please specify the SID
directly '
  'using dom-sid option.'))

-try:
-(old_dn, old_attrs) = ldap.get_entry(dn,
-['ipabaseid',
-'ipaidrangesize',
-'ipabaserid',
-'ipasecondarybaserid'])
-except errors.NotFound:
-self.obj.handle_not_found(*keys)
+if in_updated_attrs('ipanttrusteddomainsid'):
+if in_updated_attrs('ipasecondarybaserid'):
+raise errors.ValidationError(name='ID Range setup',
+error=_('Options dom_sid and secondary_rid_base
cannot '
+'be used together'))

Since we agreed to refer to options by their CLI name (--dom-sid and
--secondary-rid-base) in the other patch, it makes sense to use it
here too.



-if is_set('ipanttrusteddomainsid'):
-# Validate SID as the one of trusted domains
-
self.obj.validate_trusted_domain_sid(entry_attrs['ipanttrusteddomainsid'])

+if not in_updated_attrs('ipabaserid'):
+raise errors.ValidationError(name='ID Range setup',
+error=_('Options dom_sid and rid_base must '
+'be used together'))

Same here.


+# secondary base rid must be set if and only if base rid
is set
+if in_updated_attrs('ipasecondarybaserid') !=\
+in_updated_attrs('ipabaserid'):
+raise errors.ValidationError(name='ID Range setup',
+error=_('Options secondary_rid_base and rid_base
must '
+'be used together'))

Same here.


+dict(
+desc='Try to modify ID range %r so it has only primary
rid range set' % (testrange8),
+command=('idrange_mod', [testrange8],
+  dict(ipabaserid=testrange8_base_rid)),
+expected=errors.ValidationError(
+name='ID Range setup', error='Options
secondary_rid_base and rid_base must be used together'),
+),

And synchronize error message here too.


Thanks!

Sending the updated patch 0024.

Tomas


In patch 0024 your intention is OK, but the checking functions are not:

   is_set = lambda x: (x in entry_attrs) and (x is not None)
+in_updated_attrs = lambda x: any((x in attrs and x is not None)
+ for attrs in (entry_attrs, old_attrs))

They return True even when the attribute is None because they check if *x* is
None and not if *attrs[x]* is None. Example:

# ipa idrange-add --base-id=120 --range-size=20 --rid-base=1000
--secondary-rid-base=100 local_range

Added ID range local_range

Range name: local_range
First Posix ID of the range: 120
Number of IDs in the range: 20
First RID of the corresponding RID range: 1000
First RID of the secondary RID range: 100
Range type: local domain range

This command should be NOOP, but is not:

# ipa idrange-mod local_range --dom-sid=
ipa: ERROR: invalid 'ID Range setup': Options dom-sid and secondary-rid-base
cannot be used together

Martin

Thanks for the catch, all checking functions fixed.

Sending the complete patchset, up to date.

Tomas


I think I am being a nitpicker now (sorry), but this condition also fails if
the old_attrs has a setting, but I am removing it in a current -mod command:

# ipa idrange-add --base-id=120 --range-size=20 --rid-base=1000
--secondary-rid-base=100 local_range

Added ID range local_range

   Range name: local_range
   First Posix ID of the range: 120
   Number of IDs in the range: 20
   First RID of the corresponding RID range: 1000
   First RID of the secondary RID range: 100
   Range type: local domain range
# ipa idrange-mod local_range --dom-sid S-1-2 --secondary-rid-base=
ipa: ERROR: invalid 'ID Range setup': Options dom-sid and secondary-rid-base
cannot be used together

Martin


Yep. Ok, the command should now handle this use case as well.

Tomas
From 61fcd3db8a14a17ed5854dfb4a9f11e2bb06784e Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Wed, 20 Feb 

Re: [Freeipa-devel] [PATCHES 0024-0025] Improvements to idrange.py

2013-02-22 Thread Tomas Babej

On 02/21/2013 02:22 PM, Martin Kosek wrote:

On 02/20/2013 03:19 PM, Tomas Babej wrote:

On Wed 20 Feb 2013 02:24:03 PM CET, Alexander Bokovoy wrote:

On Wed, 20 Feb 2013, Tomas Babej wrote:

On 12/21/2012 12:15 PM, Tomas Babej wrote:

Hi,

Sending updated and rebased versions of patches 0024 and 0025.

Tomas



Sending rebased version, these got quite rotten.

Thanks for updating them.


@@ -504,25 +515,37 @@ class idrange_mod(LDAPUpdate):
 'not be found. Please specify the SID
directly '
 'using dom-sid option.'))

-try:
-(old_dn, old_attrs) = ldap.get_entry(dn,
-['ipabaseid',
-'ipaidrangesize',
-'ipabaserid',
-'ipasecondarybaserid'])
-except errors.NotFound:
-self.obj.handle_not_found(*keys)
+if in_updated_attrs('ipanttrusteddomainsid'):
+if in_updated_attrs('ipasecondarybaserid'):
+raise errors.ValidationError(name='ID Range setup',
+error=_('Options dom_sid and secondary_rid_base
cannot '
+'be used together'))

Since we agreed to refer to options by their CLI name (--dom-sid and
--secondary-rid-base) in the other patch, it makes sense to use it
here too.



-if is_set('ipanttrusteddomainsid'):
-# Validate SID as the one of trusted domains
-
self.obj.validate_trusted_domain_sid(entry_attrs['ipanttrusteddomainsid'])

+if not in_updated_attrs('ipabaserid'):
+raise errors.ValidationError(name='ID Range setup',
+error=_('Options dom_sid and rid_base must '
+'be used together'))

Same here.


+# secondary base rid must be set if and only if base rid
is set
+if in_updated_attrs('ipasecondarybaserid') !=\
+in_updated_attrs('ipabaserid'):
+raise errors.ValidationError(name='ID Range setup',
+error=_('Options secondary_rid_base and rid_base
must '
+'be used together'))

Same here.


+dict(
+desc='Try to modify ID range %r so it has only primary
rid range set' % (testrange8),
+command=('idrange_mod', [testrange8],
+  dict(ipabaserid=testrange8_base_rid)),
+expected=errors.ValidationError(
+name='ID Range setup', error='Options
secondary_rid_base and rid_base must be used together'),
+),

And synchronize error message here too.


Thanks!

Sending the updated patch 0024.

Tomas


In patch 0024 your intention is OK, but the checking functions are not:

  is_set = lambda x: (x in entry_attrs) and (x is not None)
+in_updated_attrs = lambda x: any((x in attrs and x is not None)
+ for attrs in (entry_attrs, old_attrs))

They return True even when the attribute is None because they check if *x* is
None and not if *attrs[x]* is None. Example:

# ipa idrange-add --base-id=120 --range-size=20 --rid-base=1000 
--secondary-rid-base=100 local_range

Added ID range local_range

   Range name: local_range
   First Posix ID of the range: 120
   Number of IDs in the range: 20
   First RID of the corresponding RID range: 1000
   First RID of the secondary RID range: 100
   Range type: local domain range

This command should be NOOP, but is not:

# ipa idrange-mod local_range --dom-sid=
ipa: ERROR: invalid 'ID Range setup': Options dom-sid and secondary-rid-base
cannot be used together

Martin

Thanks for the catch, all checking functions fixed.

Sending the complete patchset, up to date.

Tomas
From 61fcd3db8a14a17ed5854dfb4a9f11e2bb06784e Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Wed, 20 Feb 2013 10:50:36 +0100
Subject: [PATCH] Add trusted domain range objectclass when using idrange-mod

When modifing the idrange, one was able to add ipa NT trusted
AD domain sid without objectclass ipatrustedaddomainrange being
added. This patch fixes the issue.
---
 ipalib/plugins/idrange.py | 5 +
 1 file changed, 5 insertions(+)

diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
index 97b9d2489b51cf1fc62f49e0d3faf9674851d68a..087a1b128256063194e39bfa2369fd66af4b516f 100644
--- a/ipalib/plugins/idrange.py
+++ b/ipalib/plugins/idrange.py
@@ -531,6 +531,11 @@ class idrange_mod(LDAPUpdate):
 # perform this check only if the attribute was changed
 self.obj.validate_trusted_domain_sid(
 entry_attrs['ipanttrusteddomainsid'])
+
+   # Add trusted AD domain range object class, if it wasn't there
+if not 'ipatrustedaddomainrange' in 

Re: [Freeipa-devel] [PATCHES 0024-0025] Improvements to idrange.py

2013-02-22 Thread Martin Kosek
On 02/22/2013 03:01 PM, Tomas Babej wrote:
 On 02/21/2013 02:22 PM, Martin Kosek wrote:
 On 02/20/2013 03:19 PM, Tomas Babej wrote:
 On Wed 20 Feb 2013 02:24:03 PM CET, Alexander Bokovoy wrote:
 On Wed, 20 Feb 2013, Tomas Babej wrote:
 On 12/21/2012 12:15 PM, Tomas Babej wrote:
 Hi,

 Sending updated and rebased versions of patches 0024 and 0025.

 Tomas


 Sending rebased version, these got quite rotten.
 Thanks for updating them.

 @@ -504,25 +515,37 @@ class idrange_mod(LDAPUpdate):
  'not be found. Please specify the SID
 directly '
  'using dom-sid option.'))

 -try:
 -(old_dn, old_attrs) = ldap.get_entry(dn,
 -['ipabaseid',
 -'ipaidrangesize',
 -'ipabaserid',
 -'ipasecondarybaserid'])
 -except errors.NotFound:
 -self.obj.handle_not_found(*keys)
 +if in_updated_attrs('ipanttrusteddomainsid'):
 +if in_updated_attrs('ipasecondarybaserid'):
 +raise errors.ValidationError(name='ID Range setup',
 +error=_('Options dom_sid and secondary_rid_base
 cannot '
 +'be used together'))
 Since we agreed to refer to options by their CLI name (--dom-sid and
 --secondary-rid-base) in the other patch, it makes sense to use it
 here too.


 -if is_set('ipanttrusteddomainsid'):
 -# Validate SID as the one of trusted domains
 -
 self.obj.validate_trusted_domain_sid(entry_attrs['ipanttrusteddomainsid'])

 +if not in_updated_attrs('ipabaserid'):
 +raise errors.ValidationError(name='ID Range setup',
 +error=_('Options dom_sid and rid_base must '
 +'be used together'))
 Same here.

 +# secondary base rid must be set if and only if base rid
 is set
 +if in_updated_attrs('ipasecondarybaserid') !=\
 +in_updated_attrs('ipabaserid'):
 +raise errors.ValidationError(name='ID Range setup',
 +error=_('Options secondary_rid_base and rid_base
 must '
 +'be used together'))
 Same here.

 +dict(
 +desc='Try to modify ID range %r so it has only primary
 rid range set' % (testrange8),
 +command=('idrange_mod', [testrange8],
 +  dict(ipabaserid=testrange8_base_rid)),
 +expected=errors.ValidationError(
 +name='ID Range setup', error='Options
 secondary_rid_base and rid_base must be used together'),
 +),
 And synchronize error message here too.

 Thanks!

 Sending the updated patch 0024.

 Tomas

 In patch 0024 your intention is OK, but the checking functions are not:

   is_set = lambda x: (x in entry_attrs) and (x is not None)
 +in_updated_attrs = lambda x: any((x in attrs and x is not None)
 + for attrs in (entry_attrs, 
 old_attrs))

 They return True even when the attribute is None because they check if *x* is
 None and not if *attrs[x]* is None. Example:

 # ipa idrange-add --base-id=120 --range-size=20 --rid-base=1000
 --secondary-rid-base=100 local_range
 
 Added ID range local_range
 
Range name: local_range
First Posix ID of the range: 120
Number of IDs in the range: 20
First RID of the corresponding RID range: 1000
First RID of the secondary RID range: 100
Range type: local domain range

 This command should be NOOP, but is not:

 # ipa idrange-mod local_range --dom-sid=
 ipa: ERROR: invalid 'ID Range setup': Options dom-sid and secondary-rid-base
 cannot be used together

 Martin
 Thanks for the catch, all checking functions fixed.
 
 Sending the complete patchset, up to date.
 
 Tomas

I think I am being a nitpicker now (sorry), but this condition also fails if
the old_attrs has a setting, but I am removing it in a current -mod command:

# ipa idrange-add --base-id=120 --range-size=20 --rid-base=1000
--secondary-rid-base=100 local_range

Added ID range local_range

  Range name: local_range
  First Posix ID of the range: 120
  Number of IDs in the range: 20
  First RID of the corresponding RID range: 1000
  First RID of the secondary RID range: 100
  Range type: local domain range
# ipa idrange-mod local_range --dom-sid S-1-2 --secondary-rid-base=
ipa: ERROR: invalid 'ID Range setup': Options dom-sid and secondary-rid-base
cannot be used together

Martin

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


Re: [Freeipa-devel] [PATCHES 0024-0025] Improvements to idrange.py

2013-02-21 Thread Martin Kosek
On 02/20/2013 03:19 PM, Tomas Babej wrote:
 On Wed 20 Feb 2013 02:24:03 PM CET, Alexander Bokovoy wrote:
 On Wed, 20 Feb 2013, Tomas Babej wrote:
 On 12/21/2012 12:15 PM, Tomas Babej wrote:
 Hi,

 Sending updated and rebased versions of patches 0024 and 0025.

 Tomas


 Sending rebased version, these got quite rotten.
 Thanks for updating them.

 @@ -504,25 +515,37 @@ class idrange_mod(LDAPUpdate):
 'not be found. Please specify the SID
 directly '
 'using dom-sid option.'))

 -try:
 -(old_dn, old_attrs) = ldap.get_entry(dn,
 -['ipabaseid',
 -'ipaidrangesize',
 -'ipabaserid',
 -'ipasecondarybaserid'])
 -except errors.NotFound:
 -self.obj.handle_not_found(*keys)
 +if in_updated_attrs('ipanttrusteddomainsid'):
 +if in_updated_attrs('ipasecondarybaserid'):
 +raise errors.ValidationError(name='ID Range setup',
 +error=_('Options dom_sid and secondary_rid_base
 cannot '
 +'be used together'))
 Since we agreed to refer to options by their CLI name (--dom-sid and
 --secondary-rid-base) in the other patch, it makes sense to use it
 here too.


 -if is_set('ipanttrusteddomainsid'):
 -# Validate SID as the one of trusted domains
 -
 self.obj.validate_trusted_domain_sid(entry_attrs['ipanttrusteddomainsid'])

 +if not in_updated_attrs('ipabaserid'):
 +raise errors.ValidationError(name='ID Range setup',
 +error=_('Options dom_sid and rid_base must '
 +'be used together'))
 Same here.

 +# secondary base rid must be set if and only if base rid
 is set
 +if in_updated_attrs('ipasecondarybaserid') !=\
 +in_updated_attrs('ipabaserid'):
 +raise errors.ValidationError(name='ID Range setup',
 +error=_('Options secondary_rid_base and rid_base
 must '
 +'be used together'))
 Same here.

 +dict(
 +desc='Try to modify ID range %r so it has only primary
 rid range set' % (testrange8),
 +command=('idrange_mod', [testrange8],
 +  dict(ipabaserid=testrange8_base_rid)),
 +expected=errors.ValidationError(
 +name='ID Range setup', error='Options
 secondary_rid_base and rid_base must be used together'),
 +),
 And synchronize error message here too.

 
 Thanks!
 
 Sending the updated patch 0024.
 
 Tomas
 

In patch 0024 your intention is OK, but the checking functions are not:

 is_set = lambda x: (x in entry_attrs) and (x is not None)
+in_updated_attrs = lambda x: any((x in attrs and x is not None)
+ for attrs in (entry_attrs, old_attrs))

They return True even when the attribute is None because they check if *x* is
None and not if *attrs[x]* is None. Example:

# ipa idrange-add --base-id=120 --range-size=20 --rid-base=1000
--secondary-rid-base=100 local_range

Added ID range local_range

  Range name: local_range
  First Posix ID of the range: 120
  Number of IDs in the range: 20
  First RID of the corresponding RID range: 1000
  First RID of the secondary RID range: 100
  Range type: local domain range

This command should be NOOP, but is not:

# ipa idrange-mod local_range --dom-sid=
ipa: ERROR: invalid 'ID Range setup': Options dom-sid and secondary-rid-base
cannot be used together

Martin

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


Re: [Freeipa-devel] [PATCHES 0024-0025] Improvements to idrange.py

2013-02-20 Thread Tomas Babej

On 12/21/2012 12:15 PM, Tomas Babej wrote:

Hi,

Sending updated and rebased versions of patches 0024 and 0025.

Tomas



Sending rebased version, these got quite rotten.

Tomas

From f21b135d546678544ccf05efd587b46bba88e07a Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Fri, 21 Dec 2012 05:34:37 -0500
Subject: [PATCH] Make options checks in idrange-add/mod consistent

Both now enforce the following checks:
  - dom_sid and secondary_rid_base cannot be used together
  - rid_base must be used together if dom_rid is set
  - secondary_rid_base and rid_base must be used together
if dom_rid is not set

Unit test for third check has been added.

http://fedorahosted.org/freeipa/ticket/3170
---
 ipalib/plugins/idrange.py  | 56 +-
 tests/test_xmlrpc/test_range_plugin.py | 46 +++-
 2 files changed, 87 insertions(+), 15 deletions(-)

diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
index 445473394223f3f7c69d1e32be71d7c6944e6252..97b9d2489b51cf1fc62f49e0d3faf9674851d68a 100644
--- a/ipalib/plugins/idrange.py
+++ b/ipalib/plugins/idrange.py
@@ -402,11 +402,13 @@ class idrange_add(LDAPCreate):
 entry_attrs['objectclass'].append('ipatrustedaddomainrange')
 
 else:
+ # secondary base rid must be set if and only if base rid is set
 if is_set('ipasecondarybaserid') != is_set('ipabaserid'):
 raise errors.ValidationError(name='ID Range setup',
 error=_('Options secondary-rid-base and rid-base must '
 'be used together'))
 
+# and they must not overlap
 if is_set('ipabaserid') and is_set('ipasecondarybaserid'):
 if self.obj.are_rid_ranges_overlapping(
 entry_attrs['ipabaserid'],
@@ -483,7 +485,14 @@ class idrange_mod(LDAPUpdate):
 assert isinstance(dn, DN)
 attrs_list.append('objectclass')
 
+try:
+(old_dn, old_attrs) = ldap.get_entry(dn, ['*'])
+except errors.NotFound:
+self.obj.handle_not_found(*keys)
+
 is_set = lambda x: (x in entry_attrs) and (x is not None)
+in_updated_attrs = lambda x: any((x in attrs and x is not None)
+ for attrs in (entry_attrs, old_attrs))
 
 # This needs to stay in options since there is no
 # ipanttrusteddomainname attribute in LDAP
@@ -496,6 +505,8 @@ class idrange_mod(LDAPUpdate):
 sid = self.obj.get_trusted_domain_sid_from_name(
 options['ipanttrusteddomainname'])
 
+# we translate the name into sid so further validation can rely
+# on ipanttrusteddomainsid attribute only
 if sid is not None:
 entry_attrs['ipanttrusteddomainsid'] = sid
 else:
@@ -504,25 +515,37 @@ class idrange_mod(LDAPUpdate):
 'not be found. Please specify the SID directly '
 'using dom-sid option.'))
 
-try:
-(old_dn, old_attrs) = ldap.get_entry(dn,
-['ipabaseid',
-'ipaidrangesize',
-'ipabaserid',
-'ipasecondarybaserid'])
-except errors.NotFound:
-self.obj.handle_not_found(*keys)
+if in_updated_attrs('ipanttrusteddomainsid'):
+if in_updated_attrs('ipasecondarybaserid'):
+raise errors.ValidationError(name='ID Range setup',
+error=_('Options dom_sid and secondary_rid_base cannot '
+'be used together'))
 
-if is_set('ipanttrusteddomainsid'):
-# Validate SID as the one of trusted domains
-self.obj.validate_trusted_domain_sid(entry_attrs['ipanttrusteddomainsid'])
+if not in_updated_attrs('ipabaserid'):
+raise errors.ValidationError(name='ID Range setup',
+error=_('Options dom_sid and rid_base must '
+'be used together'))
+
+if is_set('ipanttrusteddomainsid'):
+# Validate SID as the one of trusted domains
+# perform this check only if the attribute was changed
+self.obj.validate_trusted_domain_sid(
+entry_attrs['ipanttrusteddomainsid'])
+else:
+# secondary base rid must be set if and only if base rid is set
+if in_updated_attrs('ipasecondarybaserid') !=\
+in_updated_attrs('ipabaserid'):
+raise errors.ValidationError(name='ID Range setup',
+error=_('Options secondary_rid_base and rid_base must '
+'be used together'))
 
 # ensure that primary and secondary rid ranges do not 

Re: [Freeipa-devel] [PATCHES 0024-0025] Improvements to idrange.py

2013-02-20 Thread Alexander Bokovoy

On Wed, 20 Feb 2013, Tomas Babej wrote:

On 12/21/2012 12:15 PM, Tomas Babej wrote:

Hi,

Sending updated and rebased versions of patches 0024 and 0025.

Tomas



Sending rebased version, these got quite rotten.

Thanks for updating them.


@@ -504,25 +515,37 @@ class idrange_mod(LDAPUpdate):
'not be found. Please specify the SID directly '
'using dom-sid option.'))

-try:
-(old_dn, old_attrs) = ldap.get_entry(dn,
-['ipabaseid',
-'ipaidrangesize',
-'ipabaserid',
-'ipasecondarybaserid'])
-except errors.NotFound:
-self.obj.handle_not_found(*keys)
+if in_updated_attrs('ipanttrusteddomainsid'):
+if in_updated_attrs('ipasecondarybaserid'):
+raise errors.ValidationError(name='ID Range setup',
+error=_('Options dom_sid and secondary_rid_base cannot '
+'be used together'))

Since we agreed to refer to options by their CLI name (--dom-sid and
--secondary-rid-base) in the other patch, it makes sense to use it here too.

 

-if is_set('ipanttrusteddomainsid'):
-# Validate SID as the one of trusted domains
-
self.obj.validate_trusted_domain_sid(entry_attrs['ipanttrusteddomainsid'])
+if not in_updated_attrs('ipabaserid'):
+raise errors.ValidationError(name='ID Range setup',
+error=_('Options dom_sid and rid_base must '
+'be used together'))

Same here.


+# secondary base rid must be set if and only if base rid is set
+if in_updated_attrs('ipasecondarybaserid') !=\
+in_updated_attrs('ipabaserid'):
+raise errors.ValidationError(name='ID Range setup',
+error=_('Options secondary_rid_base and rid_base must '
+'be used together'))

Same here.


+dict(
+desc='Try to modify ID range %r so it has only primary rid range 
set' % (testrange8),
+command=('idrange_mod', [testrange8],
+  dict(ipabaserid=testrange8_base_rid)),
+expected=errors.ValidationError(
+name='ID Range setup', error='Options secondary_rid_base and 
rid_base must be used together'),
+),

And synchronize error message here too.

--
/ Alexander Bokovoy

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


Re: [Freeipa-devel] [PATCHES 0024-0025] Improvements to idrange.py

2013-02-20 Thread Tomas Babej

On Wed 20 Feb 2013 02:24:03 PM CET, Alexander Bokovoy wrote:

On Wed, 20 Feb 2013, Tomas Babej wrote:

On 12/21/2012 12:15 PM, Tomas Babej wrote:

Hi,

Sending updated and rebased versions of patches 0024 and 0025.

Tomas



Sending rebased version, these got quite rotten.

Thanks for updating them.


@@ -504,25 +515,37 @@ class idrange_mod(LDAPUpdate):
'not be found. Please specify the SID
directly '
'using dom-sid option.'))

-try:
-(old_dn, old_attrs) = ldap.get_entry(dn,
-['ipabaseid',
-'ipaidrangesize',
-'ipabaserid',
-'ipasecondarybaserid'])
-except errors.NotFound:
-self.obj.handle_not_found(*keys)
+if in_updated_attrs('ipanttrusteddomainsid'):
+if in_updated_attrs('ipasecondarybaserid'):
+raise errors.ValidationError(name='ID Range setup',
+error=_('Options dom_sid and secondary_rid_base
cannot '
+'be used together'))

Since we agreed to refer to options by their CLI name (--dom-sid and
--secondary-rid-base) in the other patch, it makes sense to use it
here too.



-if is_set('ipanttrusteddomainsid'):
-# Validate SID as the one of trusted domains
-
self.obj.validate_trusted_domain_sid(entry_attrs['ipanttrusteddomainsid'])

+if not in_updated_attrs('ipabaserid'):
+raise errors.ValidationError(name='ID Range setup',
+error=_('Options dom_sid and rid_base must '
+'be used together'))

Same here.


+# secondary base rid must be set if and only if base rid
is set
+if in_updated_attrs('ipasecondarybaserid') !=\
+in_updated_attrs('ipabaserid'):
+raise errors.ValidationError(name='ID Range setup',
+error=_('Options secondary_rid_base and rid_base
must '
+'be used together'))

Same here.


+dict(
+desc='Try to modify ID range %r so it has only primary
rid range set' % (testrange8),
+command=('idrange_mod', [testrange8],
+  dict(ipabaserid=testrange8_base_rid)),
+expected=errors.ValidationError(
+name='ID Range setup', error='Options
secondary_rid_base and rid_base must be used together'),
+),

And synchronize error message here too.



Thanks!

Sending the updated patch 0024.

Tomas


From d36c9aa5fb7dd044e212de4cd17adbc044d14777 Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Fri, 21 Dec 2012 05:34:37 -0500
Subject: [PATCH] Make options checks in idrange-add/mod consistent

Both now enforce the following checks:
  - dom_sid and secondary_rid_base cannot be used together
  - rid_base must be used together if dom_rid is set
  - secondary_rid_base and rid_base must be used together
if dom_rid is not set

Unit test for third check has been added.

http://fedorahosted.org/freeipa/ticket/3170
---
 ipalib/plugins/idrange.py  | 56 +-
 tests/test_xmlrpc/test_range_plugin.py | 46 +++-
 2 files changed, 87 insertions(+), 15 deletions(-)

diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
index 445473394223f3f7c69d1e32be71d7c6944e6252..6a4c7a5b5de7971abf374572cd3bdadfaae93651 100644
--- a/ipalib/plugins/idrange.py
+++ b/ipalib/plugins/idrange.py
@@ -402,11 +402,13 @@ class idrange_add(LDAPCreate):
 entry_attrs['objectclass'].append('ipatrustedaddomainrange')
 
 else:
+ # secondary base rid must be set if and only if base rid is set
 if is_set('ipasecondarybaserid') != is_set('ipabaserid'):
 raise errors.ValidationError(name='ID Range setup',
 error=_('Options secondary-rid-base and rid-base must '
 'be used together'))
 
+# and they must not overlap
 if is_set('ipabaserid') and is_set('ipasecondarybaserid'):
 if self.obj.are_rid_ranges_overlapping(
 entry_attrs['ipabaserid'],
@@ -483,7 +485,14 @@ class idrange_mod(LDAPUpdate):
 assert isinstance(dn, DN)
 attrs_list.append('objectclass')
 
+try:
+(old_dn, old_attrs) = ldap.get_entry(dn, ['*'])
+except errors.NotFound:
+self.obj.handle_not_found(*keys)
+
 is_set = lambda x: (x in entry_attrs) and (x is not None)
+in_updated_attrs = lambda x: any((x in attrs and x is not None)
+ for attrs in (entry_attrs, old_attrs))
 
 # This needs to stay in options since there is no
 # ipanttrusteddomainname attribute in LDAP
@@ -496,6 

[Freeipa-devel] [PATCHES 0024-0025] Improvements to idrange.py

2012-12-21 Thread Tomas Babej

Hi,

Sending updated and rebased versions of patches 0024 and 0025.

Tomas



From 6d4903a1c5e255929cdbce2a67d79c6e44b1 Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Fri, 21 Dec 2012 05:34:37 -0500
Subject: [PATCH] Make options checks in idrange-add/mod consistent

Both now enforce the following checks:
  - dom_sid and secondary_rid_base cannot be used together
  - rid_base must be used together if dom_rid is set
  - secondary_rid_base and rid_base must be used together
if dom_rid is not set

Unit test for third check has been added.

http://fedorahosted.org/freeipa/ticket/3170
---
 ipalib/plugins/idrange.py  | 47 +-
 tests/test_xmlrpc/test_range_plugin.py | 46 -
 2 files changed, 80 insertions(+), 13 deletions(-)

diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
index 84e1057ac6b59b8ad99882a54e3288897338c978..911d5a2563e8264ad398830618e13abdab09d94c 100644
--- a/ipalib/plugins/idrange.py
+++ b/ipalib/plugins/idrange.py
@@ -353,11 +353,13 @@ class idrange_add(LDAPCreate):
 entry_attrs['objectclass'].append('ipatrustedaddomainrange')
 
 else:
+# secondary base rid must be set if and only if base rid is set
 if is_set('ipasecondarybaserid') != is_set('ipabaserid'):
 raise errors.ValidationError(name='ID Range setup',
 error=_('Options secondary_rid_base and rid_base must '
 'be used together'))
 
+# and they must not overlap
 if is_set('ipabaserid') and is_set('ipasecondarybaserid'):
 if self.obj.are_rid_ranges_overlapping(
 entry_attrs['ipabaserid'],
@@ -434,24 +436,40 @@ class idrange_mod(LDAPUpdate):
 assert isinstance(dn, DN)
 attrs_list.append('objectclass')
 
-is_set = lambda x: (x in entry_attrs) and (x is not None)
-
 try:
-(old_dn, old_attrs) = ldap.get_entry(dn,
-['ipabaseid',
-'ipaidrangesize',
-'ipabaserid',
-'ipasecondarybaserid'])
+(old_dn, old_attrs) = ldap.get_entry(dn, ['*'])
 except errors.NotFound:
 self.obj.handle_not_found(*keys)
 
-if is_set('ipanttrusteddomainsid'):
-# Validate SID as the one of trusted domains
-self.obj.validate_trusted_domain_sid(options['ipanttrusteddomainsid'])
+is_set = lambda x: (x in entry_attrs) and (x is not None)
+in_updated_attrs = lambda x : any((x in attrs and x is not None)
+  for attrs in (entry_attrs, old_attrs))
+
+if in_updated_attrs('ipanttrusteddomainsid'):
+if in_updated_attrs('ipasecondarybaserid'):
+raise errors.ValidationError(name='ID Range setup',
+error=_('Options dom_sid and secondary_rid_base cannot '
+'be used together'))
+
+if not in_updated_attrs('ipabaserid'):
+raise errors.ValidationError(name='ID Range setup',
+error=_('Options dom_sid and rid_base must '
+'be used together'))
+
+if is_set('ipanttrusteddomainsid'):
+# Validate SID as the one of trusted domains
+# perform this check only if the attribute was changed
+self.obj.validate_trusted_domain_sid(entry_attrs['ipanttrusteddomainsid'])
+else:
+# secondary base rid must be set if and only if base rid is set
+if (in_updated_attrs('ipasecondarybaserid') != in_updated_attrs('ipabaserid')):
+raise errors.ValidationError(name='ID Range setup',
+error=_('Options secondary_rid_base and rid_base must '
+'be used together'))
 
 # ensure that primary and secondary rid ranges do not overlap
-if all((base in entry_attrs) or (base in old_attrs)
-for base in ('ipabaserid', 'ipasecondarybaserid')):
+if all(in_updated_attrs(base)
+   for base in ('ipabaserid', 'ipasecondarybaserid')):
 
 # make sure we are working with updated attributes
 rid_range_attributes = ('ipabaserid', 'ipasecondarybaserid', 'ipaidrangesize')
@@ -471,14 +489,19 @@ class idrange_mod(LDAPUpdate):
 error=_(Primary RID range and secondary RID range
   cannot overlap))
 
+# check whether ids are in modified range
 old_base_id = int(old_attrs.get('ipabaseid', [0])[0])
 old_range_size = int(old_attrs.get('ipaidrangesize', [0])[0])
 new_base_id = entry_attrs.get('ipabaseid')
+
 if new_base_id is not None: