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