[Freeipa-devel] [PATCH] 0074 validate SID for trusted domain when adding/modifying ID range

[Martin Kosek]

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 '''<text>''', 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