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

[Alexander Bokovoy]

On Fri, 15 Feb 2013, Tomas Babej wrote:
>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.).
Ok, thanks.

>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.
Agreed.

>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.
Please do.


>Updated patch attached (error messages refactored).
ACK.

-- 
/ Alexander Bokovoy