Re: [Freeipa-devel] [PATCH] 0074 validate SID for trusted domain when adding/modifying ID range
On 09/20/2012 02:31 PM, Alexander Bokovoy wrote:
> On Thu, 20 Sep 2012, Martin Kosek wrote:
>> On 09/20/2012 01:58 PM, Alexander Bokovoy wrote:
>>> On Thu, 20 Sep 2012, Petr Viktorin wrote:
On 09/20/2012 12:12 PM, Martin Kosek wrote:
> On 09/20/2012 11:42 AM, Alexander Bokovoy wrote:
>> Hi,
>>
>> On Thu, 20 Sep 2012, Martin Kosek wrote:
>>> On 09/19/2012 06:19 PM, Alexander Bokovoy wrote:
Hi,
This patch adds validation of SID for trusted domain when adding or
modifying ID range for the domain. We only allow creating ranges for
trusted domains when the trust is already established -- the default
range is created automatically right after the trust is added.
https://fedorahosted.org/freeipa/ticket/3087
>>>
>>> Basic functionality looks OK, but I saw few issues with exception
>>> formatting:
>>>
>>> diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
>>> index
>>> efa906428aa58c670bc4af63b10c88123dda5b65..4750c1d6716bd69045d53f32ae1836f44e70b03b
>>>
>>>
>>>
>>> 100644
>>> --- a/ipalib/plugins/idrange.py
>>> +++ b/ipalib/plugins/idrange.py
>>> @@ -26,6 +26,12 @@ from ipapython import ipautil
>>> from ipalib import util
>>> from ipapython.dn import DN
>>>
>>> +if api.env.in_server and api.env.context in ['lite', 'server']:
>>> +try:
>>> +import ipaserver.dcerpc
>>> +_dcerpc_bindings_installed = True
>>> +except Exception, e:
>>> +_dcerpc_bindings_installed = False
>>>
>>>
>>> Variable "e" is not used, so it can be removed.
>> Then Exception, e should be omitted completely :)
>
> As per PEP8, "except Exception:" is preffered over bare "except:" as
> otherwise
> it would also catch SystemExit or KeyboardInterrupt.
You should use the most specific exception you want to handle. In this case
it's probably ImportError.
>>> New patch is attached.
>>>
>>
>> The patch looks OK, I would just also like to have the rest of the name=_('ID
>> Range setup') messages fixed so that we don't print confusing errors:
>>
>> $ git grep "ID Range setup" ipalib/
>> ipalib/plugins/idrange.py:raise
>> errors.ValidationError(name=_('ID Range setup'),
>> ipalib/plugins/idrange.py:raise
>> errors.ValidationError(name=_('ID Range setup'),
>> ipalib/plugins/idrange.py:raise
>> errors.ValidationError(name=_('ID Range setup'),
> Yep. Done.
>
>
ACK. Pushed to master, ipa-3-0.
Martin
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0074 validate SID for trusted domain when adding/modifying ID range
On Thu, 20 Sep 2012, Martin Kosek wrote:
On 09/20/2012 01:58 PM, Alexander Bokovoy wrote:
On Thu, 20 Sep 2012, Petr Viktorin wrote:
On 09/20/2012 12:12 PM, Martin Kosek wrote:
On 09/20/2012 11:42 AM, Alexander Bokovoy wrote:
Hi,
On Thu, 20 Sep 2012, Martin Kosek wrote:
On 09/19/2012 06:19 PM, Alexander Bokovoy wrote:
Hi,
This patch adds validation of SID for trusted domain when adding or
modifying ID range for the domain. We only allow creating ranges for
trusted domains when the trust is already established -- the default
range is created automatically right after the trust is added.
https://fedorahosted.org/freeipa/ticket/3087
Basic functionality looks OK, but I saw few issues with exception formatting:
diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
index
efa906428aa58c670bc4af63b10c88123dda5b65..4750c1d6716bd69045d53f32ae1836f44e70b03b
100644
--- a/ipalib/plugins/idrange.py
+++ b/ipalib/plugins/idrange.py
@@ -26,6 +26,12 @@ from ipapython import ipautil
from ipalib import util
from ipapython.dn import DN
+if api.env.in_server and api.env.context in ['lite', 'server']:
+try:
+import ipaserver.dcerpc
+_dcerpc_bindings_installed = True
+except Exception, e:
+_dcerpc_bindings_installed = False
Variable "e" is not used, so it can be removed.
Then Exception, e should be omitted completely :)
As per PEP8, "except Exception:" is preffered over bare "except:" as otherwise
it would also catch SystemExit or KeyboardInterrupt.
You should use the most specific exception you want to handle. In this case
it's probably ImportError.
New patch is attached.
The patch looks OK, I would just also like to have the rest of the name=_('ID
Range setup') messages fixed so that we don't print confusing errors:
$ git grep "ID Range setup" ipalib/
ipalib/plugins/idrange.py:raise
errors.ValidationError(name=_('ID Range setup'),
ipalib/plugins/idrange.py:raise
errors.ValidationError(name=_('ID Range setup'),
ipalib/plugins/idrange.py:raise
errors.ValidationError(name=_('ID Range setup'),
Yep. Done.
--
/ Alexander Bokovoy
>From 86fef992edd0f6e7b5c818c774352067e90e02a3 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy
Date: Wed, 19 Sep 2012 19:09:22 +0300
Subject: [PATCH 1/4] validate SID for trusted domain when adding/modifying ID
range
https://fedorahosted.org/freeipa/ticket/3087
---
ipalib/plugins/idrange.py | 31 ---
1 file changed, 28 insertions(+), 3 deletions(-)
diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
index
ee50613bbaeb70aecf830ad480773a253f88a136..8f2d4efdc0463e7d81cd72fba7769e38dc0c638b
100644
--- a/ipalib/plugins/idrange.py
+++ b/ipalib/plugins/idrange.py
@@ -26,6 +26,12 @@ from ipapython import ipautil
from ipalib import util
from ipapython.dn import DN
+if api.env.in_server and api.env.context in ['lite', 'server']:
+try:
+import ipaserver.dcerpc
+_dcerpc_bindings_installed = True
+except ImportError:
+_dcerpc_bindings_installed = False
__doc__ = _("""
ID ranges
@@ -249,6 +255,18 @@ class idrange(LDAPObject):
error=_('range modification leaving objects with ID out '
'of the defined range is not allowed'))
+def validate_trusted_domain_sid(self, sid):
+if not _dcerpc_bindings_installed:
+raise errors.NotFound(reason=_('Cannot perform SID validation
without Samba 4 support installed. '
+ 'Make sure you have installed server-trust-ad
sub-package of IPA on the server'))
+domain_validator = ipaserver.dcerpc.DomainValidator(self.api)
+if not domain_validator.is_configured():
+raise errors.NotFound(reason=_('Cross-realm trusts are not
configured. '
+ 'Make sure you have run ipa-adtrust-install on the
IPA server first'))
+if not domain_validator.is_trusted_sid_valid(sid):
+raise errors.ValidationError(name='domain SID',
+ error=_('SID is not recognized as a valid SID for a trusted
domain'))
+
class idrange_add(LDAPCreate):
__doc__ = _("""
Add new ID range.
@@ -278,19 +296,22 @@ class idrange_add(LDAPCreate):
if 'ipanttrusteddomainsid' in options:
if 'ipasecondarybaserid' in options:
-raise errors.ValidationError(name=_('ID Range setup'),
+raise errors.ValidationError(name='ID Range setup',
error=_('Options dom_sid and secondary_rid_base cannot ' \
'be used together'))
if 'ipabaserid' not in options:
-raise errors.ValidationError(name=_('ID Range setup'),
+raise errors.ValidationError(name='ID Range setup',
error=_('Options dom_sid and rid_base must ' \
'be used together'))
+# Validate SID
Re: [Freeipa-devel] [PATCH] 0074 validate SID for trusted domain when adding/modifying ID range
On 09/20/2012 01:58 PM, Alexander Bokovoy wrote:
> On Thu, 20 Sep 2012, Petr Viktorin wrote:
>> On 09/20/2012 12:12 PM, Martin Kosek wrote:
>>> On 09/20/2012 11:42 AM, Alexander Bokovoy wrote:
Hi,
On Thu, 20 Sep 2012, Martin Kosek wrote:
> On 09/19/2012 06:19 PM, Alexander Bokovoy wrote:
>> Hi,
>>
>> This patch adds validation of SID for trusted domain when adding or
>> modifying ID range for the domain. We only allow creating ranges for
>> trusted domains when the trust is already established -- the default
>> range is created automatically right after the trust is added.
>>
>> https://fedorahosted.org/freeipa/ticket/3087
>>
>
> Basic functionality looks OK, but I saw few issues with exception
> formatting:
>
> diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
> index
> efa906428aa58c670bc4af63b10c88123dda5b65..4750c1d6716bd69045d53f32ae1836f44e70b03b
>
>
> 100644
> --- a/ipalib/plugins/idrange.py
> +++ b/ipalib/plugins/idrange.py
> @@ -26,6 +26,12 @@ from ipapython import ipautil
> from ipalib import util
> from ipapython.dn import DN
>
> +if api.env.in_server and api.env.context in ['lite', 'server']:
> +try:
> +import ipaserver.dcerpc
> +_dcerpc_bindings_installed = True
> +except Exception, e:
> +_dcerpc_bindings_installed = False
>
>
> Variable "e" is not used, so it can be removed.
Then Exception, e should be omitted completely :)
>>>
>>> As per PEP8, "except Exception:" is preffered over bare "except:" as
>>> otherwise
>>> it would also catch SystemExit or KeyboardInterrupt.
>>
>> You should use the most specific exception you want to handle. In this case
>> it's probably ImportError.
> New patch is attached.
>
The patch looks OK, I would just also like to have the rest of the name=_('ID
Range setup') messages fixed so that we don't print confusing errors:
$ git grep "ID Range setup" ipalib/
ipalib/plugins/idrange.py:raise
errors.ValidationError(name=_('ID Range setup'),
ipalib/plugins/idrange.py:raise
errors.ValidationError(name=_('ID Range setup'),
ipalib/plugins/idrange.py:raise
errors.ValidationError(name=_('ID Range setup'),
Martin
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0074 validate SID for trusted domain when adding/modifying ID range
On Thu, 20 Sep 2012, Petr Viktorin wrote:
On 09/20/2012 12:12 PM, Martin Kosek wrote:
On 09/20/2012 11:42 AM, Alexander Bokovoy wrote:
Hi,
On Thu, 20 Sep 2012, Martin Kosek wrote:
On 09/19/2012 06:19 PM, Alexander Bokovoy wrote:
Hi,
This patch adds validation of SID for trusted domain when adding or
modifying ID range for the domain. We only allow creating ranges for
trusted domains when the trust is already established -- the default
range is created automatically right after the trust is added.
https://fedorahosted.org/freeipa/ticket/3087
Basic functionality looks OK, but I saw few issues with exception formatting:
diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
index
efa906428aa58c670bc4af63b10c88123dda5b65..4750c1d6716bd69045d53f32ae1836f44e70b03b
100644
--- a/ipalib/plugins/idrange.py
+++ b/ipalib/plugins/idrange.py
@@ -26,6 +26,12 @@ from ipapython import ipautil
from ipalib import util
from ipapython.dn import DN
+if api.env.in_server and api.env.context in ['lite', 'server']:
+try:
+import ipaserver.dcerpc
+_dcerpc_bindings_installed = True
+except Exception, e:
+_dcerpc_bindings_installed = False
Variable "e" is not used, so it can be removed.
Then Exception, e should be omitted completely :)
As per PEP8, "except Exception:" is preffered over bare "except:" as otherwise
it would also catch SystemExit or KeyboardInterrupt.
You should use the most specific exception you want to handle. In
this case it's probably ImportError.
New patch is attached.
--
/ Alexander Bokovoy
>From 4ddcad0b54e18339581a7aec042f42bec5bc7b48 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy
Date: Wed, 19 Sep 2012 19:09:22 +0300
Subject: [PATCH 1/4] validate SID for trusted domain when adding/modifying ID
range
https://fedorahosted.org/freeipa/ticket/3087
---
ipalib/plugins/idrange.py | 25 +
1 file changed, 25 insertions(+)
diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
index
ee50613bbaeb70aecf830ad480773a253f88a136..4ef3559aca0ef5314e44b727e97106866db94cda
100644
--- a/ipalib/plugins/idrange.py
+++ b/ipalib/plugins/idrange.py
@@ -26,6 +26,12 @@ from ipapython import ipautil
from ipalib import util
from ipapython.dn import DN
+if api.env.in_server and api.env.context in ['lite', 'server']:
+try:
+import ipaserver.dcerpc
+_dcerpc_bindings_installed = True
+except ImportError:
+_dcerpc_bindings_installed = False
__doc__ = _("""
ID ranges
@@ -249,6 +255,18 @@ class idrange(LDAPObject):
error=_('range modification leaving objects with ID out '
'of the defined range is not allowed'))
+def validate_trusted_domain_sid(self, sid):
+if not _dcerpc_bindings_installed:
+raise errors.NotFound(reason=_('Cannot perform SID validation
without Samba 4 support installed. '
+ 'Make sure you have installed server-trust-ad
sub-package of IPA on the server'))
+domain_validator = ipaserver.dcerpc.DomainValidator(self.api)
+if not domain_validator.is_configured():
+raise errors.NotFound(reason=_('Cross-realm trusts are not
configured. '
+ 'Make sure you have run ipa-adtrust-install on the
IPA server first'))
+if not domain_validator.is_trusted_sid_valid(sid):
+raise errors.ValidationError(name='domain SID',
+ error=_('SID is not recognized as a valid SID for a trusted
domain'))
+
class idrange_add(LDAPCreate):
__doc__ = _("""
Add new ID range.
@@ -287,6 +305,9 @@ class idrange_add(LDAPCreate):
error=_('Options dom_sid and rid_base must ' \
'be used together'))
+# Validate SID as the one of trusted domains
+
self.obj.validate_trusted_domain_sid(options['ipanttrusteddomainsid'])
+# Finally, add trusted AD domain range object class
entry_attrs['objectclass'].append('ipatrustedaddomainrange')
else:
if (('ipasecondarybaserid' in options) != ('ipabaserid' in
options)):
@@ -366,6 +387,10 @@ class idrange_mod(LDAPUpdate):
except errors.NotFound:
self.obj.handle_not_found(*keys)
+if 'ipanttrusteddomainsid' in options:
+# Validate SID as the one of trusted domains
+
self.obj.validate_trusted_domain_sid(options['ipanttrusteddomainsid'])
+
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')
--
1.7.12
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0074 validate SID for trusted domain when adding/modifying ID range
On 09/20/2012 12:12 PM, Martin Kosek wrote:
On 09/20/2012 11:42 AM, Alexander Bokovoy wrote:
Hi,
On Thu, 20 Sep 2012, Martin Kosek wrote:
On 09/19/2012 06:19 PM, Alexander Bokovoy wrote:
Hi,
This patch adds validation of SID for trusted domain when adding or
modifying ID range for the domain. We only allow creating ranges for
trusted domains when the trust is already established -- the default
range is created automatically right after the trust is added.
https://fedorahosted.org/freeipa/ticket/3087
Basic functionality looks OK, but I saw few issues with exception formatting:
diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
index
efa906428aa58c670bc4af63b10c88123dda5b65..4750c1d6716bd69045d53f32ae1836f44e70b03b
100644
--- a/ipalib/plugins/idrange.py
+++ b/ipalib/plugins/idrange.py
@@ -26,6 +26,12 @@ from ipapython import ipautil
from ipalib import util
from ipapython.dn import DN
+if api.env.in_server and api.env.context in ['lite', 'server']:
+try:
+import ipaserver.dcerpc
+_dcerpc_bindings_installed = True
+except Exception, e:
+_dcerpc_bindings_installed = False
Variable "e" is not used, so it can be removed.
Then Exception, e should be omitted completely :)
As per PEP8, "except Exception:" is preffered over bare "except:" as otherwise
it would also catch SystemExit or KeyboardInterrupt.
You should use the most specific exception you want to handle. In this
case it's probably ImportError.
__doc__ = _("""
ID ranges
@@ -137,6 +143,21 @@ user. RIDs are unique in a domain, 32bit values and are
used for users and
groups.
""")
+def validate_trusted_domain_sid(self, sid):
"self" is not needed in the list of attributes, this is not a class method.
I'm using self.api inside. While api is singleton and accessible
globally, I'd prefer passing it explicitly. So may be I'll change that
to 'api'.
Looks like to hack to me. I would either define this function as a method of
idrange class (as I did with handle_iparangetype) and then use self.api or use
"api" directly as a function parameter and not make assumptions what "self" may
be.
+if not _dcerpc_bindings_installed:
+raise errors.NotFound(name=_('ID Range setup'),
+ reason=_('''Cannot perform SID validation without Samba 4
support installed.
+ Make sure you have installed server-trust-ad
sub-package of IPA on the server'''))
Improperly formatted exception:
1) NotFound error does not use "name" param, maybe you wanted to use
ValidationError?
2) The text will be improperly formatted - since you used '', the
indentation will be in text:
ipa: ERROR: Cannot perform SID validation without Samba 4 support installed.
Make sure you have installed server-trust-ad
sub-package of IPA on the server
Will fix it.
Also, I know this was discussed before, but using gettext in a name attribute
of ValidationError will cause improperly formatted exception:
# ipa idrange-add foo --base-id=1000 --range-size=100 --dom-sid=foo
ipa: ERROR: invalid Gettext('ID Range setup', domain='ipa', localedir=None):
Options dom_sid and rid_base must be used together
The problem is, that "name" param is printer as %r, thus you would need to
coerce it to unicode to make it better.
I'd rather fix our printing code and Gettext() usage to properly give
out the original string rather than swallow limitations we put on
ourselves. Wider use will be, more translations will be needed and names
will need to be translated as well.
ValidationError has this format: _('invalid %(name)r: %(error)s'). AFAIU, name
parameter was intended to contain only a name of attribute or option where the
validation failed, i.e. a non-translable string. So this was the reason why we
print it as "%r". This pattern is followed in the rest of the plugins.
In your case, I think using name="dom_sid" would be the preferred use of
ValidationError.
Martin
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
--
Petr³
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0074 validate SID for trusted domain when adding/modifying ID range
On 09/20/2012 11:42 AM, Alexander Bokovoy wrote:
> Hi,
>
> On Thu, 20 Sep 2012, Martin Kosek wrote:
>> On 09/19/2012 06:19 PM, Alexander Bokovoy wrote:
>>> Hi,
>>>
>>> This patch adds validation of SID for trusted domain when adding or
>>> modifying ID range for the domain. We only allow creating ranges for
>>> trusted domains when the trust is already established -- the default
>>> range is created automatically right after the trust is added.
>>>
>>> https://fedorahosted.org/freeipa/ticket/3087
>>>
>>
>> Basic functionality looks OK, but I saw few issues with exception formatting:
>>
>> diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
>> index
>> efa906428aa58c670bc4af63b10c88123dda5b65..4750c1d6716bd69045d53f32ae1836f44e70b03b
>>
>> 100644
>> --- a/ipalib/plugins/idrange.py
>> +++ b/ipalib/plugins/idrange.py
>> @@ -26,6 +26,12 @@ from ipapython import ipautil
>> from ipalib import util
>> from ipapython.dn import DN
>>
>> +if api.env.in_server and api.env.context in ['lite', 'server']:
>> +try:
>> +import ipaserver.dcerpc
>> +_dcerpc_bindings_installed = True
>> +except Exception, e:
>> +_dcerpc_bindings_installed = False
>>
>>
>> Variable "e" is not used, so it can be removed.
> Then Exception, e should be omitted completely :)
As per PEP8, "except Exception:" is preffered over bare "except:" as otherwise
it would also catch SystemExit or KeyboardInterrupt.
>
>
>>
>> __doc__ = _("""
>> ID ranges
>> @@ -137,6 +143,21 @@ user. RIDs are unique in a domain, 32bit values and are
>> used for users and
>> groups.
>> """)
>>
>> +def validate_trusted_domain_sid(self, sid):
>>
>> "self" is not needed in the list of attributes, this is not a class method.
> I'm using self.api inside. While api is singleton and accessible
> globally, I'd prefer passing it explicitly. So may be I'll change that
> to 'api'.
Looks like to hack to me. I would either define this function as a method of
idrange class (as I did with handle_iparangetype) and then use self.api or use
"api" directly as a function parameter and not make assumptions what "self" may
be.
>
>> +if not _dcerpc_bindings_installed:
>> +raise errors.NotFound(name=_('ID Range setup'),
>> + reason=_('''Cannot perform SID validation without Samba 4
>> support installed.
>> + Make sure you have installed server-trust-ad
>> sub-package of IPA on the server'''))
>>
>> Improperly formatted exception:
>> 1) NotFound error does not use "name" param, maybe you wanted to use
>> ValidationError?
>> 2) The text will be improperly formatted - since you used '', the
>> indentation will be in text:
>>
>> ipa: ERROR: Cannot perform SID validation without Samba 4 support installed.
>> Make sure you have installed server-trust-ad
>> sub-package of IPA on the server
> Will fix it.
>
>>
>> Also, I know this was discussed before, but using gettext in a name attribute
>> of ValidationError will cause improperly formatted exception:
>>
>> # ipa idrange-add foo --base-id=1000 --range-size=100 --dom-sid=foo
>> ipa: ERROR: invalid Gettext('ID Range setup', domain='ipa', localedir=None):
>> Options dom_sid and rid_base must be used together
>>
>> The problem is, that "name" param is printer as %r, thus you would need to
>> coerce it to unicode to make it better.
> I'd rather fix our printing code and Gettext() usage to properly give
> out the original string rather than swallow limitations we put on
> ourselves. Wider use will be, more translations will be needed and names
> will need to be translated as well.
ValidationError has this format: _('invalid %(name)r: %(error)s'). AFAIU, name
parameter was intended to contain only a name of attribute or option where the
validation failed, i.e. a non-translable string. So this was the reason why we
print it as "%r". This pattern is followed in the rest of the plugins.
In your case, I think using name="dom_sid" would be the preferred use of
ValidationError.
Martin
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0074 validate SID for trusted domain when adding/modifying ID range
Hi,
On Thu, 20 Sep 2012, Martin Kosek wrote:
On 09/19/2012 06:19 PM, Alexander Bokovoy wrote:
Hi,
This patch adds validation of SID for trusted domain when adding or
modifying ID range for the domain. We only allow creating ranges for
trusted domains when the trust is already established -- the default
range is created automatically right after the trust is added.
https://fedorahosted.org/freeipa/ticket/3087
Basic functionality looks OK, but I saw few issues with exception formatting:
diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
index
efa906428aa58c670bc4af63b10c88123dda5b65..4750c1d6716bd69045d53f32ae1836f44e70b03b
100644
--- a/ipalib/plugins/idrange.py
+++ b/ipalib/plugins/idrange.py
@@ -26,6 +26,12 @@ from ipapython import ipautil
from ipalib import util
from ipapython.dn import DN
+if api.env.in_server and api.env.context in ['lite', 'server']:
+try:
+import ipaserver.dcerpc
+_dcerpc_bindings_installed = True
+except Exception, e:
+_dcerpc_bindings_installed = False
Variable "e" is not used, so it can be removed.
Then Exception, e should be omitted completely :)
__doc__ = _("""
ID ranges
@@ -137,6 +143,21 @@ user. RIDs are unique in a domain, 32bit values and are
used for users and
groups.
""")
+def validate_trusted_domain_sid(self, sid):
"self" is not needed in the list of attributes, this is not a class method.
I'm using self.api inside. While api is singleton and accessible
globally, I'd prefer passing it explicitly. So may be I'll change that
to 'api'.
+if not _dcerpc_bindings_installed:
+raise errors.NotFound(name=_('ID Range setup'),
+ reason=_('''Cannot perform SID validation without Samba 4
support installed.
+ Make sure you have installed server-trust-ad
sub-package of IPA on the server'''))
Improperly formatted exception:
1) NotFound error does not use "name" param, maybe you wanted to use
ValidationError?
2) The text will be improperly formatted - since you used '', the
indentation will be in text:
ipa: ERROR: Cannot perform SID validation without Samba 4 support installed.
Make sure you have installed server-trust-ad
sub-package of IPA on the server
Will fix it.
Also, I know this was discussed before, but using gettext in a name attribute
of ValidationError will cause improperly formatted exception:
# ipa idrange-add foo --base-id=1000 --range-size=100 --dom-sid=foo
ipa: ERROR: invalid Gettext('ID Range setup', domain='ipa', localedir=None):
Options dom_sid and rid_base must be used together
The problem is, that "name" param is printer as %r, thus you would need to
coerce it to unicode to make it better.
I'd rather fix our printing code and Gettext() usage to properly give
out the original string rather than swallow limitations we put on
ourselves. Wider use will be, more translations will be needed and names
will need to be translated as well.
+domain_validator = ipaserver.dcerpc.DomainValidator(self.api)
+if not domain_validator.is_configured():
+raise errors.NotFound(name=_('ID Range setup'),
+ reason=_('''Cross-realm trusts are not configured..
+ Make sure you have run ipa-adtrust-install on the
IPA server first'''))
Same issues:
# ipa idrange-add foo --base-id=1000 --range-size=100 --dom-sid=foo
--rid-base=1000
ipa: ERROR: Cross-realm trusts are not configured..
Make sure you have run ipa-adtrust-install on the IPA
server first
+if not domain_validator.is_trusted_sid_valid(sid):
+raise errors.ValidationError(name=_('ID Range setup'),
+ error=_('SID is not recognized as a valid SID from a trusted
domain'))
+
+
Same issues:
ipa: ERROR: invalid Gettext('ID Range setup', domain='ipa', localedir=None):
SID is not recognized as a valid SID from a trusted domain
I'll fix formatting.
Thanks!
--
/ Alexander Bokovoy
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0074 validate SID for trusted domain when adding/modifying ID range
On 09/19/2012 06:19 PM, Alexander Bokovoy wrote:
> Hi,
>
> This patch adds validation of SID for trusted domain when adding or
> modifying ID range for the domain. We only allow creating ranges for
> trusted domains when the trust is already established -- the default
> range is created automatically right after the trust is added.
>
> https://fedorahosted.org/freeipa/ticket/3087
>
Basic functionality looks OK, but I saw few issues with exception formatting:
diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
index
efa906428aa58c670bc4af63b10c88123dda5b65..4750c1d6716bd69045d53f32ae1836f44e70b03b
100644
--- a/ipalib/plugins/idrange.py
+++ b/ipalib/plugins/idrange.py
@@ -26,6 +26,12 @@ from ipapython import ipautil
from ipalib import util
from ipapython.dn import DN
+if api.env.in_server and api.env.context in ['lite', 'server']:
+try:
+import ipaserver.dcerpc
+_dcerpc_bindings_installed = True
+except Exception, e:
+_dcerpc_bindings_installed = False
Variable "e" is not used, so it can be removed.
__doc__ = _("""
ID ranges
@@ -137,6 +143,21 @@ user. RIDs are unique in a domain, 32bit values and are
used for users and
groups.
""")
+def validate_trusted_domain_sid(self, sid):
"self" is not needed in the list of attributes, this is not a class method.
+if not _dcerpc_bindings_installed:
+raise errors.NotFound(name=_('ID Range setup'),
+ reason=_('''Cannot perform SID validation without Samba 4
support installed.
+ Make sure you have installed server-trust-ad
sub-package of IPA on the server'''))
Improperly formatted exception:
1) NotFound error does not use "name" param, maybe you wanted to use
ValidationError?
2) The text will be improperly formatted - since you used '', the
indentation will be in text:
ipa: ERROR: Cannot perform SID validation without Samba 4 support installed.
Make sure you have installed server-trust-ad
sub-package of IPA on the server
Also, I know this was discussed before, but using gettext in a name attribute
of ValidationError will cause improperly formatted exception:
# ipa idrange-add foo --base-id=1000 --range-size=100 --dom-sid=foo
ipa: ERROR: invalid Gettext('ID Range setup', domain='ipa', localedir=None):
Options dom_sid and rid_base must be used together
The problem is, that "name" param is printer as %r, thus you would need to
coerce it to unicode to make it better.
+domain_validator = ipaserver.dcerpc.DomainValidator(self.api)
+if not domain_validator.is_configured():
+raise errors.NotFound(name=_('ID Range setup'),
+ reason=_('''Cross-realm trusts are not configured..
+ Make sure you have run ipa-adtrust-install on the
IPA server first'''))
Same issues:
# ipa idrange-add foo --base-id=1000 --range-size=100 --dom-sid=foo
--rid-base=1000
ipa: ERROR: Cross-realm trusts are not configured..
Make sure you have run ipa-adtrust-install on the IPA
server first
+if not domain_validator.is_trusted_sid_valid(sid):
+raise errors.ValidationError(name=_('ID Range setup'),
+ error=_('SID is not recognized as a valid SID from a trusted
domain'))
+
+
Same issues:
ipa: ERROR: invalid Gettext('ID Range setup', domain='ipa', localedir=None):
SID is not recognized as a valid SID from a trusted domain
Martin
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 0074 validate SID for trusted domain when adding/modifying ID range
Hi,
This patch adds validation of SID for trusted domain when adding or
modifying ID range for the domain. We only allow creating ranges for
trusted domains when the trust is already established -- the default
range is created automatically right after the trust is added.
https://fedorahosted.org/freeipa/ticket/3087
--
/ Alexander Bokovoy
>From c8859d449b65be67841c96c81f7f64f8c27b06b1 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy
Date: Wed, 19 Sep 2012 19:09:22 +0300
Subject: [PATCH] validate SID for trusted domain when adding/modifying ID
range
https://fedorahosted.org/freeipa/ticket/3087
---
ipalib/plugins/idrange.py | 28
1 file changed, 28 insertions(+)
diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
index
efa906428aa58c670bc4af63b10c88123dda5b65..4750c1d6716bd69045d53f32ae1836f44e70b03b
100644
--- a/ipalib/plugins/idrange.py
+++ b/ipalib/plugins/idrange.py
@@ -26,6 +26,12 @@ from ipapython import ipautil
from ipalib import util
from ipapython.dn import DN
+if api.env.in_server and api.env.context in ['lite', 'server']:
+try:
+import ipaserver.dcerpc
+_dcerpc_bindings_installed = True
+except Exception, e:
+_dcerpc_bindings_installed = False
__doc__ = _("""
ID ranges
@@ -137,6 +143,21 @@ user. RIDs are unique in a domain, 32bit values and are
used for users and
groups.
""")
+def validate_trusted_domain_sid(self, sid):
+if not _dcerpc_bindings_installed:
+raise errors.NotFound(name=_('ID Range setup'),
+ reason=_('''Cannot perform SID validation without Samba 4
support installed.
+ Make sure you have installed server-trust-ad
sub-package of IPA on the server'''))
+domain_validator = ipaserver.dcerpc.DomainValidator(self.api)
+if not domain_validator.is_configured():
+raise errors.NotFound(name=_('ID Range setup'),
+ reason=_('''Cross-realm trusts are not configured..
+ Make sure you have run ipa-adtrust-install on the
IPA server first'''))
+if not domain_validator.is_trusted_sid_valid(sid):
+raise errors.ValidationError(name=_('ID Range setup'),
+ error=_('SID is not recognized as a valid SID from a trusted
domain'))
+
+
class idrange(LDAPObject):
"""
Range object.
@@ -287,6 +308,9 @@ class idrange_add(LDAPCreate):
error=_('Options dom_sid and rid_base must ' \
'be used together'))
+# Validate SID as the one of trusted domains
+validate_trusted_domain_sid(self, options['ipanttrusteddomainsid'])
+# Finally, add trusted AD domain range object class
entry_attrs['objectclass'].append('ipatrustedaddomainrange')
else:
if (('ipasecondarybaserid' in options) != ('ipabaserid' in
options)):
@@ -366,6 +390,10 @@ class idrange_mod(LDAPUpdate):
except errors.NotFound:
self.obj.handle_not_found(*keys)
+if 'ipanttrusteddomainsid' in options:
+# Validate SID as the one of trusted domains
+validate_trusted_domain_sid(self, options['ipanttrusteddomainsid'])
+
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')
--
1.7.12
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
