[Freeipa-devel] [PATCH 0052] Only specify the ipatokenuniqueid default in the add operation

[Nathaniel McCallum]

On Thu, 2014-05-22 at 17:13 +0200, Petr Vobornik wrote:
> On 22.5.2014 17:00, Nathaniel McCallum wrote:
> > On Thu, 2014-05-22 at 10:53 -0400, Nathaniel McCallum wrote:
> >> On Thu, 2014-05-22 at 16:45 +0200, Petr Viktorin wrote:
> >>> On 05/22/2014 04:12 PM, Nathaniel McCallum wrote:
> >>>> On Tue, 2014-05-13 at 12:55 -0400, Nathaniel McCallum wrote:
> >>>>> On Tue, 2014-05-13 at 16:47 +0200, Jan Cholasta wrote:
> >>>>>> On 12.5.2014 20:50, Nathaniel McCallum wrote:
> >>>>>>> On Mon, 2014-05-12 at 18:40 +0200, Misnyovszki Adam wrote:
> >>>>>>>> On Tue, 06 May 2014 11:46:14 -0400
> >>>>>>>> Nathaniel McCallum <npmccallum at redhat.com> wrote:
> >>>>>>>>
> >>>>>>>>> On Tue, 2014-05-06 at 11:38 -0400, Nathaniel McCallum wrote:
> >>>>>>>>>> On Tue, 2014-05-06 at 17:34 +0200, Petr Vobornik wrote:
> >>>>>>>>>>> On 6.5.2014 17:13, Nathaniel McCallum wrote:
> >>>>>>>>>>>> On Tue, 2014-05-06 at 17:04 +0200, Petr Vobornik wrote:
> >>>>>>>>>>>>> On 6.5.2014 16:51, Nathaniel McCallum wrote:
> >>>>>>>>>>>>>> Specifying the default in the LDAP Object causes the
> >>>>>>>>>>>>>> parameter to be specified for non-add operations. This is
> >>>>>>>>>>>>>> especially problematic when performing the modify operation
> >>>>>>>>>>>>>> as it causes the primary key to change for every modification.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> https://fedorahosted.org/freeipa/ticket/4227
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> shouldn't removal of `autofill=True,` be enough?
> >>>>>>>>>>>>
> >>>>>>>>>>>> Removing autofill=True results in the default not being used
> >>>>>>>>>>>> for the otptoken-add operation. That may be a different bug
> >>>>>>>>>>>> (I'm not sure what the expectation of autofill is).
> >>>>>>>>>>>>
> >>>>>>>>>>>> Nathaniel
> >>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> Seems to work form me with:
> >>>>>>>>>>>
> >>>>>>>>>>> diff --git a/ipalib/plugins/otptoken.py
> >>>>>>>>>>> b/ipalib/plugins/otptoken.py index f68ea7d..623f1f1 100644
> >>>>>>>>>>> --- a/ipalib/plugins/otptoken.py
> >>>>>>>>>>> +++ b/ipalib/plugins/otptoken.py
> >>>>>>>>>>> @@ -121,9 +121,7 @@ class otptoken(LDAPObject):
> >>>>>>>>>>>                  cli_name='id',
> >>>>>>>>>>>                  label=_('Unique ID'),
> >>>>>>>>>>>                  default_from=lambda: unicode(uuid.uuid4()),
> >>>>>>>>>>> -            autofill=True,
> >>>>>>>>>>>                  primary_key=True,
> >>>>>>>>>>> -            flags=('optional_create'),
> >>>>>>>>>>>              ),
> >>>>>>>>>>>              StrEnum('type?',
> >>>>>>>>>>>                  label=_('Type'),
> >>>>>>>>>>
> >>>>>>>>>> Doing this causes the ipa otptoken-add command to prompt for the
> >>>>>>>>>> Unique ID. This may be the desired behavior, but it is not how it
> >>>>>>>>>> worked previously (no prompt).
> >>>>>>>>>
> >>>>>>>>> Here is an alternate patch for this second approach. I have no strong
> >>>>>>>>> opinion on the correct behavior here.
> >>>>>>>>>
> >>>>>>>>> Nathaniel
> >>>>>>>>
> >>>>>>>> IMO you should update API.txt with ./makeapi
> >>>>>>>
> >>>>>>> Running ./makeapi results in no changes to API.txt.
> >>>>>>
> >>>>>> This is not right, there *are* changes in the API and build fails for me
> >>>>>> becase API.txt is not updated.
> >>>>>
> >>>>> I think maybe I ran it from the wrong branch. Fixed.
> >>>>
> >>>> I still need a review of this. It is pretty trivial.
> >>>>
> >>>> Nathaniel
> >>>
> >>> This still prompts for the unique ID on add:
> >>>
> >>> $ ipa otptoken-add
> >>> Unique ID [25cb3aa9-db19-40f8-acf4-33ef65ca863c]:
> >>>
> >>> I don't think that's the intended behavior.
> >>
> >> Hence the alternate patch (0052a). If we don't want to prompt, we'll
> >> need to use the first patch (0052). I have no strong opinion on the
> >> correct behavior and I am fine with merging either patch.
> >
> > Attached is the non-alternate (0052) with the api updated.
> >
> > Nathaniel
> 
> IMO 52a is better if used by hand and it keeps code cleaner. It might 
> not be ideal though if used from a script because of nonexistent 
> --unattended/-u option which would disable prompt (set 
> env.interactive=False ?).

It only prompts if the id is not specified. If you are using it in a
script you can just create your own random UUID and specify it. This is
exactly what I do in the import script patch.

Nathaniel