[Freeipa-devel] [PATCH 0030] Add option to specify SID using domain name to idrange-add/mod

Fri Feb 15 11:29:49 UTC 2013

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0030-4-Add-option-to-specify-SID-using-domain-name-to-idran.patch
Type: text/x-patch
Size: 11844 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130215/688e12c1/attachment.bin>