[Freeipa-devel] [PATCH 0078] Enable QR code display by default in otptoken-add

[Petr Vobornik]

On 13.11.2014 19:12, Petr Viktorin wrote:
> On 11/13/2014 06:02 PM, Nathaniel McCallum wrote:
>> On Thu, 2014-11-13 at 16:57 +0100, Petr Viktorin wrote:
>>> On 11/13/2014 04:40 PM, Petr Vobornik wrote:
>>>> On 13.11.2014 16:19, Nathaniel McCallum wrote:
>>>>>>>
>>>>>>> Like you, I like #2 the best. Attached is an implementation.
>>>>>>
>>>>>> I like --no-qrcode as well.
>>>>>>
>>>>>> Should we also keep qrcode as 'no_option' to maintain API
>>>>>> compatibility
>>>>>> (but not CLI)?
>>>>>
>>>>> I don't think it is necessary. It only makes sense to specify --qrcode
>>>>> in an interactive session.
>>>>>
>>>>
>>>> Makes sense.
>>>>
>>>> ACK
>>>>
>>>> Not pushing yet to give time for NACK if anybody doesn't agree with the
>>>> API change.
>>>
>>> Hold on, what is happening here?
>>>
>>> Aren't all clients since 4.0 sending the qrcode option to the server?
>>> We absolutely can not break backwards compatibility with released
>>> versions.
>>> We also should not break the CLI. Just make it a no-op option, and say
>>> it's deprecated in the doc.
>>
>> As I understand the current behavior, the qrcode option is *not* sent to
>> the server by default in any scenario.
>
> Nope, defaults are filled in by the client. (And also on the server if
> they're still missing; it's part of the common validation.)

IMHO this is quite unfortunate behavior which may also fail horribly if 
there is a newer client and an older server -> backwards compatibility 
is on API level, not CLI level. Defaults should be filled by server, not 
a client.  We should seriously reconsider the design of our CLI. But 
that's for different, future discussion.

That's said and given the circumstances, it is easier and cleaner to 
return the --qrcode back as no_param now than to deal with potential 
future issues.

>
> You can try it out, actually:
>
> $ ipa -vv otptoken-add
> ipa: INFO: trying https://vm-175.idm.lab.eng.brq.redhat.com/ipa/json
> ipa: INFO: Forwarding 'otptoken_add' to json server
> 'https://vm-175.idm.lab.eng.brq.redhat.com/ipa/json'
> ipa: INFO: Request: {
>      "id": 0,
>      "method": "otptoken_add",
>      "params": [
>          [
>              null
>          ],
>          {
>              "all": false,
>              "ipatokenhotpcounter": 0,
>              "ipatokenotpalgorithm": "sha1",
>              "ipatokenotpdigits": 6,
>              "ipatokenotpkey":
> "5\ufffdK\ufffd1\u000e\ufffd7,\ufffd_\ufffd\ufffd.0\ufffdM\ufffd\u0016\ufffd",
>
>              "ipatokentotpclockoffset": 0,
>              "ipatokentotptimestep": 30,
>              "no_members": false,
>              "qrcode": false,
>              "raw": false,
>              "type": "totp",
>              "version": "2.108"
>          }
>      ]
> }
> ipa: INFO: Response: {
>      "error": null,
>      "id": 0,
>      "principal": "admin at IDM.LAB.ENG.BRQ.REDHAT.COM",
> ...
>
-- 
Petr Vobornik