On Fri, 2014-11-14 at 17:20 +0100, Petr Viktorin wrote: > On 11/14/2014 02:25 PM, Petr Vobornik wrote: > > On 14.11.2014 14:08, Petr Viktorin wrote: > >> On 11/14/2014 01:18 PM, Petr Vobornik wrote: > >> [...] > >>>> > >>>> 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. > >> > >> You can't use a newer client with an older server, you get a > >> VersionError in that case. > > > > And that's bad because, IMHO, this case may be more common that a newer > > server and an older client, e.g., RHEL 6.5 server and Fedora 21 client. > > > >> > >> Feel free to file a ticket. But yes, redesigning the API is not exactly > >> a priority. > >> > >>> 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. > >> > >> What's the reason to break the CLI by making it no_param? > > > > Sorry I meant, no_option, there is no no_param. Because it returns it > > back to Nathaniel's argument about interactive session and I agree with > > it. Why would we have both --no-qrcode and --qrcode options available on > > CLI? > > It breaks the CLI, and for that there should be a better reason than "it > looks bad". The CLI is an interface as well.
The attached patch retains --qrcode but specified as no_option. It is marked as deprecated.
From 51be0dcf3003a992909da935a0d2529b8768900e Mon Sep 17 00:00:00 2001 From: Nathaniel McCallum <npmccal...@redhat.com> Date: Thu, 6 Nov 2014 15:30:13 -0500 Subject: [PATCH] Enable QR code display by default in otptoken-add This is possible because python-qrcode's output now fits in a standard terminal. Also, update ipa-otp-import and otptoken-add-yubikey to disable QR code output as it doesn't make sense in these contexts. https://fedorahosted.org/freeipa/ticket/4703 --- API.txt | 3 ++- VERSION | 4 ++-- ipalib/plugins/otptoken.py | 5 +++-- ipalib/plugins/otptoken_yubikey.py | 1 + ipaserver/install/ipa_otptoken_import.py | 2 +- 5 files changed, 9 insertions(+), 6 deletions(-) diff --git a/API.txt b/API.txt index 0000491d7a76fd1d2d50208d314d1600839ce295..2a63f1e2349f0df69433fa7cb742e269cd42d79f 100644 --- a/API.txt +++ b/API.txt @@ -2592,7 +2592,7 @@ output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDA output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None) output: PrimaryKey('value', None, None) command: otptoken_add -args: 1,22,3 +args: 1,23,3 arg: Str('ipatokenuniqueid', attribute=True, cli_name='id', multivalue=False, primary_key=True, required=False) option: Str('addattr*', cli_name='addattr', exclude='webui') option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui') @@ -2611,6 +2611,7 @@ option: Int('ipatokentotpclockoffset', attribute=True, autofill=True, cli_name=' option: Int('ipatokentotptimestep', attribute=True, autofill=True, cli_name='interval', default=30, minvalue=5, multivalue=False, required=False) option: Str('ipatokenvendor', attribute=True, cli_name='vendor', multivalue=False, required=False) option: Flag('no_members', autofill=True, default=False, exclude='webui') +option: Flag('no_qrcode', autofill=True, default=False) option: Flag('qrcode?', autofill=True, default=False) option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui') option: Str('setattr*', cli_name='setattr', exclude='webui') diff --git a/VERSION b/VERSION index b0d41e5e1ec59ddefbdcccf588b97bac2ff798ee..bae782a4ec4333f8fdb610465a7b9ea3877c990e 100644 --- a/VERSION +++ b/VERSION @@ -90,5 +90,5 @@ IPA_DATA_VERSION=20100614120000 # # ######################################################## IPA_API_VERSION_MAJOR=2 -IPA_API_VERSION_MINOR=108 -# Last change: pvoborni - manage authorization of keytab operations +IPA_API_VERSION_MINOR=109 +# Last change: npmccallum - display qrcode by default diff --git a/ipalib/plugins/otptoken.py b/ipalib/plugins/otptoken.py index f48feeee0502992f1b5fed4f342cace1c404624b..f0850854f98e84e44acdcef311225220ac0129a3 100644 --- a/ipalib/plugins/otptoken.py +++ b/ipalib/plugins/otptoken.py @@ -268,7 +268,8 @@ class otptoken_add(LDAPCreate): msg_summary = _('Added OTP token "%(value)s"') takes_options = LDAPCreate.takes_options + ( - Flag('qrcode?', label=_('Display QR code')), + Flag('qrcode?', label=_('(deprecated)'), flags=('no_option')), + Flag('no_qrcode', label=_('Do not display QR code'), default=False), ) has_output_params = LDAPCreate.has_output_params + ( @@ -348,7 +349,7 @@ class otptoken_add(LDAPCreate): rv = super(otptoken_add, self).output_for_cli(textui, output, *args, **options) # Print QR code to terminal if specified - if uri and options.get('qrcode', False): + if uri and not options.get('no_qrcode', False): print "\n" qr = qrcode.QRCode() qr.add_data(uri) diff --git a/ipalib/plugins/otptoken_yubikey.py b/ipalib/plugins/otptoken_yubikey.py index e70ddb6e42b5ea34d7ebecb252d6bbd73ac64d03..7095887ac7cdf5d4b7d0d30edc6cab0222246664 100644 --- a/ipalib/plugins/otptoken_yubikey.py +++ b/ipalib/plugins/otptoken_yubikey.py @@ -124,6 +124,7 @@ class otptoken_add_yubikey(Command): ipatokenotpalgorithm=u'sha1', ipatokenhotpcounter=0, ipatokenotpkey=key, + no_qrcode=True, **options) # Suppress values we don't want to return. diff --git a/ipaserver/install/ipa_otptoken_import.py b/ipaserver/install/ipa_otptoken_import.py index 31a6902014b8e3b2aafb3ba98a4190dc2059a3e7..b78aba93a2edc987450d921c87ea4f61b014b419 100644 --- a/ipaserver/install/ipa_otptoken_import.py +++ b/ipaserver/install/ipa_otptoken_import.py @@ -517,7 +517,7 @@ class OTPTokenImport(admintool.AdminTool): # Parse tokens for keypkg in self.doc.getKeyPackages(): try: - api.Command.otptoken_add(keypkg.id, **keypkg.options) + api.Command.otptoken_add(keypkg.id, no_qrcode=True, **keypkg.options) except Exception as e: self.log.warn("Error adding token: %s", e) else: -- 2.1.0
_______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
