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": "ad...@idm.lab.eng.brq.redhat.com",
...

--
Petr Vobornik

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to