On 02/14/2013 05:37 PM, Alexander Bokovoy wrote:
On Thu, 14 Feb 2013, Tomas Babej wrote:
+ Str('ipanttrusteddomainname?',
+ cli_name='dom_name',
+ flags=('no_search', 'virtual_attribute'),
+ label=_('Name of the trusted domain'),
+ ),
New options is added but API.txt wasn't changed. As result, 'make rpms'
does not work.

Could you please fix the patch and re-send it?

Sorry about that.

Updated patch attached.
I have one small question regarding use of dom_sid/dom_name.

If both dom_sid and dom_name were specified, failing to resolve dom_name
would force command to raise exception.

I'm not sure this is right behavior. Probably we should detect that both
dom_sid and dom_name were specified and bail out earlier so that only
one of them is accepted. That would be clearer to users, wouldn't it
Sure, I definitely agree on that point. I added the check to idrange-add and idrange-mod. Also, the patch needed a rebase to apply on the current master.
I tried to play with various scenarious and one thing I noticed is that we refer dom_sid and dom_name in the error messages as they are named internally. CLI replaces underscore by hyphen (_ -> -) and therefore
this error message becomes wrong -- you cannot specify --dom_sid, this
option is unknown to CLI.

In Web UI this would also mean an error message pointing to non-existing
option. Perhaps it would be reasonable to name options '--name' and
'--sid'? We don't have any clash there:
-------------------------------------------------------------------------
# ipa idrange-mod --help
Usage: ipa [global-options] idrange-mod NAME [options]

Positional arguments:
  NAME                  Range name

Options:
  -h, --help            show this help message and exit
  --base-id=INT         First Posix ID of the range
  --range-size=INT      Number of IDs in the range
  --rid-base=INT        First RID of the corresponding RID range
  --secondary-rid-base=INT
                        First RID of the secondary RID range
  --dom-sid=STR         Domain SID of the trusted domain
  --dom-name=STR        Name of the trusted domain
  --setattr=STR         Set an attribute to a name/value pair. Format is
attr=value. For multi-valued attributes, the command
                        replaces the values already present.
  --addattr=STR         Add an attribute/value pair. Format is
attr=value. The
                        attribute must be part of the schema.
  --delattr=STR         Delete an attribute/value pair. The option willbe
                        evaluated last, after all sets and adds.
  --rights              Display the access rights of this entry(requires
                        --all). See ipa man page for details.
--all Retrieve and print all attributes from the server.
                        Affects command output.
--raw Print entries as stored on the server. Only affects
                        output format.
-------------------------------------------------------------------------

So, if --name and --sid were used, an error message would become
----------------------------------------------------------------------
# ipa idrange-mod AD.LAN_id_range --dom-name foo.bar ipa: ERROR: invalid 'ID Range setup': SID for the specified trusted
domain name could not be found. Please specify the SID directly using
--sid option.
----------------------------------------------------------------------


Additionally, there is an error when SID for an object within the domain
is specified. Last RID of the SID represents an object within the domain
and we generally need to be careful allowing it in the place where
domain SID is specified:

# ipa idrange-mod AD.LAN_id_range --dom-sid S-1-5-21-3502988750-125904550-3683905862-1
-----------------------------------
Modified ID range "AD.LAN_id_range"
-----------------------------------
  Range name: AD.LAN_id_range
  First Posix ID of the range: 1442800000
  Number of IDs in the range: 200000
  First RID of the corresponding RID range: 0
Domain SID of the trusted domain: S-1-5-21-3502988750-125904550-3683905862-1
  Range type: Active Directory domain range

Now this range is completely unusable due to the fact that there is no
way to match the domain SID against the range.

I think we need to make the check against established trusts more
strict and only allow exact match.

1.) Regarding dom_sid and dom_name options, we do not have to change their internal names to get more user-friendly error messages. These are hardcoded strings, and not generated from internal names of the options. I followed the naming convention already set in the file, but you're right, the current state is somewhat confusing to the end user. I changed all the error messages so that they refer to hyphen-versions of the options (not only dom_sid/dom_name but also rid_base, etc.).

I considered renaming the options to --sid and --name. However, although --sid is pretty self-explanatory, --name could be quite confusing, as ID range has name of its own. Furthermore, this would break some documentation / other references, and since refactoring of the error messages described above solves our
issue here, I'd vote for that solution.

2.) Exact match against estabilished trusts - this is a nice catch! However, as far as I understand this is not something introduced by my patch, and it would not be transparent to include the fix here. If you agree,
I'll create a new ticket in Trac.

Updated patch attached (error messages refactored).

Tomas
>From 374a88152688c821fe8293cc067ad7449bf12536 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Mon, 4 Feb 2013 08:33:53 -0500
Subject: [PATCH] Add option to specify SID using domain name to
 idrange-add/mod

When adding/modifying an ID range for a trusted domain, the newly
added option --dom-name can be used. This looks up SID of the
trusted domain in LDAP and therefore the user is not required
to write it down in CLI. If the lookup fails, error message
asking the user to specify the SID manually is shown.

https://fedorahosted.org/freeipa/ticket/3133
---
 API.txt                   |  6 ++-
 ipalib/plugins/idrange.py | 94 ++++++++++++++++++++++++++++++++++++++++-------
 ipaserver/dcerpc.py       | 10 +++++
 3 files changed, 95 insertions(+), 15 deletions(-)

diff --git a/API.txt b/API.txt
index d1913022b180cd0922f98931ad6030cb0555a6c0..5219c51be62862c43ebe9396147ff220b33605c7 100644
--- a/API.txt
+++ b/API.txt
@@ -1885,13 +1885,14 @@ command: i18n_messages
 args: 0,0,1
 output: Output('messages', <type 'dict'>, None)
 command: idrange_add
-args: 1,11,3
+args: 1,12,3
 arg: Str('cn', attribute=True, cli_name='name', multivalue=False, primary_key=True, required=True)
 option: Str('addattr*', cli_name='addattr', exclude='webui')
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
 option: Int('ipabaseid', attribute=True, cli_name='base_id', multivalue=False, required=True)
 option: Int('ipabaserid', attribute=True, cli_name='rid_base', multivalue=False, required=False)
 option: Int('ipaidrangesize', attribute=True, cli_name='range_size', multivalue=False, required=True)
+option: Str('ipanttrusteddomainname', attribute=False, cli_name='dom_name', multivalue=False, required=False)
 option: Str('ipanttrusteddomainsid', attribute=True, cli_name='dom_sid', multivalue=False, required=False)
 option: Str('iparangetype', attribute=True, cli_name='iparangetype', multivalue=False, required=False)
 option: Int('ipasecondarybaserid', attribute=True, cli_name='secondary_rid_base', multivalue=False, required=False)
@@ -1929,7 +1930,7 @@ output: ListOfEntries('result', (<type 'list'>, <type 'tuple'>), Gettext('A list
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
 output: Output('truncated', <type 'bool'>, None)
 command: idrange_mod
-args: 1,13,3
+args: 1,14,3
 arg: Str('cn', attribute=True, cli_name='name', multivalue=False, primary_key=True, query=True, required=True)
 option: Str('addattr*', cli_name='addattr', exclude='webui')
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
@@ -1937,6 +1938,7 @@ option: Str('delattr*', cli_name='delattr', exclude='webui')
 option: Int('ipabaseid', attribute=True, autofill=False, cli_name='base_id', multivalue=False, required=False)
 option: Int('ipabaserid', attribute=True, autofill=False, cli_name='rid_base', multivalue=False, required=False)
 option: Int('ipaidrangesize', attribute=True, autofill=False, cli_name='range_size', multivalue=False, required=False)
+option: Str('ipanttrusteddomainname', attribute=False, autofill=False, cli_name='dom_name', multivalue=False, required=False)
 option: Str('ipanttrusteddomainsid', attribute=True, autofill=False, cli_name='dom_sid', multivalue=False, required=False)
 option: Str('iparangetype', attribute=True, autofill=False, cli_name='iparangetype', multivalue=False, required=False)
 option: Int('ipasecondarybaserid', attribute=True, autofill=False, cli_name='secondary_rid_base', multivalue=False, required=False)
diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
index 84e1057ac6b59b8ad99882a54e3288897338c978..445473394223f3f7c69d1e32be71d7c6944e6252 100644
--- a/ipalib/plugins/idrange.py
+++ b/ipalib/plugins/idrange.py
@@ -57,7 +57,7 @@ Additionally an ID range of the local domain may set
 
 and an ID range of a trusted domain must set
  - rid-base: the first RID of the corresponding RID range
- - dom_sid: domain SID of the trusted domain
+ - sid: domain SID of the trusted domain
 
 
 
@@ -197,6 +197,11 @@ class idrange(LDAPObject):
             cli_name='dom_sid',
             label=_('Domain SID of the trusted domain'),
         ),
+        Str('ipanttrusteddomainname?',
+            cli_name='dom_name',
+            flags=('no_search', 'virtual_attribute'),
+            label=_('Name of the trusted domain'),
+        ),
         Str('iparangetype?',
             label=_('Range type'),
             flags=['no_option'],
@@ -265,17 +270,42 @@ 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):
+    def get_domain_validator(self):
         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'))
+            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'))
+            raise errors.NotFound(reason=_('Cross-realm trusts are not '
+                'configured. Make sure you have run ipa-adtrust-install '
+                'on the IPA server first'))
+
+        return domain_validator
+
+    def validate_trusted_domain_sid(self, sid):
+
+        domain_validator = self.get_domain_validator()
+
         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'))
+                  error=_('SID is not recognized as a valid SID for a '
+                          'trusted domain'))
+
+    def get_trusted_domain_sid_from_name(self, name):
+        """ Returns unicode string representation for given trusted domain name
+        or None if SID forthe given trusted domain name could not be found."""
+
+        domain_validator = self.get_domain_validator()
+
+        sid = domain_validator.get_sid_from_domain_name(name)
+
+        if sid is not None:
+            sid = unicode(sid)
+
+        return sid
 
     # checks that primary and secondary rid ranges do not overlap
     def are_rid_ranges_overlapping(self, rid_base, secondary_rid_base, size):
@@ -336,26 +366,45 @@ class idrange_add(LDAPCreate):
 
         is_set = lambda x: (x in entry_attrs) and (x is not None)
 
+        # This needs to stay in options since there is no
+        # ipanttrusteddomainname attribute in LDAP
+        if 'ipanttrusteddomainname' in options:
+            if is_set('ipanttrusteddomainsid'):
+                raise errors.ValidationError(name='ID Range setup',
+                    error=_('Options dom-sid and dom-name '
+                            'cannot be used together'))
+
+            sid = self.obj.get_trusted_domain_sid_from_name(
+                options['ipanttrusteddomainname'])
+
+            if sid is not None:
+                entry_attrs['ipanttrusteddomainsid'] = sid
+            else:
+                raise errors.ValidationError(name='ID Range setup',
+                    error=_('SID for the specified trusted domain name could '
+                            'not be found. Please specify the SID directly '
+                            'using dom-sid option.'))
+
         if is_set('ipanttrusteddomainsid'):
             if is_set('ipasecondarybaserid'):
                 raise errors.ValidationError(name='ID Range setup',
-                    error=_('Options dom_sid and secondary_rid_base cannot '
-                            'be used together'))
+                    error=_('Options dom-sid/dom-name and secondary-rid-base '
+                            'cannot be used together'))
 
             if not is_set('ipabaserid'):
                 raise errors.ValidationError(name='ID Range setup',
-                    error=_('Options dom_sid and rid_base must '
+                    error=_('Options dom-sid/dom-name and rid-base must '
                             'be used together'))
 
             # Validate SID as the one of trusted domains
-            self.obj.validate_trusted_domain_sid(options['ipanttrusteddomainsid'])
+            self.obj.validate_trusted_domain_sid(entry_attrs['ipanttrusteddomainsid'])
             # Finally, add trusted AD domain range object class
             entry_attrs['objectclass'].append('ipatrustedaddomainrange')
 
         else:
             if is_set('ipasecondarybaserid') != is_set('ipabaserid'):
                 raise errors.ValidationError(name='ID Range setup',
-                    error=_('Options secondary_rid_base and rid_base must '
+                    error=_('Options secondary-rid-base and rid-base must '
                             'be used together'))
 
             if is_set('ipabaserid') and is_set('ipasecondarybaserid'):
@@ -436,6 +485,25 @@ class idrange_mod(LDAPUpdate):
 
         is_set = lambda x: (x in entry_attrs) and (x is not None)
 
+        # This needs to stay in options since there is no
+        # ipanttrusteddomainname attribute in LDAP
+        if 'ipanttrusteddomainname' in options:
+            if is_set('ipanttrusteddomainsid'):
+                raise errors.ValidationError(name='ID Range setup',
+                    error=_('Options dom-sid and dom-name '
+                            'cannot be used together'))
+
+            sid = self.obj.get_trusted_domain_sid_from_name(
+                options['ipanttrusteddomainname'])
+
+            if sid is not None:
+                entry_attrs['ipanttrusteddomainsid'] = sid
+            else:
+                raise errors.ValidationError(name='ID Range setup',
+                    error=_('SID for the specified trusted domain name could '
+                            'not be found. Please specify the SID directly '
+                            'using dom-sid option.'))
+
         try:
             (old_dn, old_attrs) = ldap.get_entry(dn,
                                                 ['ipabaseid',
@@ -447,7 +515,7 @@ class idrange_mod(LDAPUpdate):
 
         if is_set('ipanttrusteddomainsid'):
             # Validate SID as the one of trusted domains
-            self.obj.validate_trusted_domain_sid(options['ipanttrusteddomainsid'])
+            self.obj.validate_trusted_domain_sid(entry_attrs['ipanttrusteddomainsid'])
 
         # ensure that primary and secondary rid ranges do not overlap
         if all((base in entry_attrs) or (base in old_attrs)
diff --git a/ipaserver/dcerpc.py b/ipaserver/dcerpc.py
index 6243ebbb92e501dfb0919353d38a7a0af64183c0..b471bccee414281e26eaaf404b59fb3268d37112 100644
--- a/ipaserver/dcerpc.py
+++ b/ipaserver/dcerpc.py
@@ -204,6 +204,16 @@ class DomainValidator(object):
         else:
             return True
 
+    def get_sid_from_domain_name(self, name):
+        """Returns binary representation of SID for the trusted domain name
+           or None if name is not in the list of trusted domains."""
+
+        domains = self.get_trusted_domains()
+        if name in domains:
+            return domains[name][1]
+        else:
+            return None
+
     def get_trusted_domain_objects(self, domain=None, flatname=None, filter="",
             attrs=None, scope=_ldap.SCOPE_SUBTREE, basedn=None):
         """
-- 
1.7.11.7

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

Reply via email to