Re: [Freeipa-devel] [PATCH] otptoken_yubikey, append CR by default and add a option for not doing so
Dne 17.4.2015 v 13:58 Nathaniel McCallum napsal(a): On Thu, 2015-04-16 at 09:12 +0200, Jan Cholasta wrote: Dne 9.4.2015 v 15:11 Luc de Louw napsal(a): On 04/09/2015 02:28 PM, Jan Cholasta wrote: Let's say you now introduce --no-cr flag. What if we decide to change the default to False? How would you then change the option/API? You would have to add --cr flag. That was the point - some clients would send ct flag, some no_cr and there would have to be special handling. It is more flexible IMO to just use something like --cr=TRUE|FALSE with TRUE being the default I would say --append-cr=TRUE|FALSE with no default, meaning do not add the flag to the config at all. I though the idea was to append the CR by default, i.e. --append-cr=TRUE|FALSE with TRUE being the default. If you want to hardcode the default into the plugin, there is no benefit in using Bool over Flag, because Flag is actually a Bool with hardcoded default value. I actually started with a bool, default=True. I had the problem that the Default value was ignored, the value was None. Changing the default behavior is IMHO bad anyway does not matter if Bool or Flag. +1 Please advise what is you wish to be implemented :-) That depends. Is there a difference between do not set APPEND_CR ticket flag and set APPEND_CR ticket flag to false? For YubiKey hardware the flag is either present (true) or absent (false). This flag controls whether or not the carriage return is sent (present) or not (absent). The param should be a Flag then. -- Jan Cholasta -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] otptoken_yubikey, append CR by default and add a option for not doing so
On 04/21/2015 01:08 PM, Jan Cholasta wrote: The param should be a Flag then. Okay, will work on that on the week end then. Thanks, Luc -- Luc de Louw Senior Linux Consultant Red Hat GmbH Am Treptower Park 75, 2nd floor D-12435 Berlin Email: ldel...@redhat.com Cell Germany: +49 162 413 29 64 Red Hat GmbH, http://www.de.redhat.com/ Registered seat: Grasbrunn, Commercial register: Amtsgericht Muenchen, HRB 153243, Managing Directors: Charles Cachera, Michael Cunningham, Michael O'Neill, Charles Peters -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] otptoken_yubikey, append CR by default and add a option for not doing so
On Thu, 2015-04-16 at 09:12 +0200, Jan Cholasta wrote: Dne 9.4.2015 v 15:11 Luc de Louw napsal(a): On 04/09/2015 02:28 PM, Jan Cholasta wrote: Let's say you now introduce --no-cr flag. What if we decide to change the default to False? How would you then change the option/API? You would have to add --cr flag. That was the point - some clients would send ct flag, some no_cr and there would have to be special handling. It is more flexible IMO to just use something like --cr=TRUE|FALSE with TRUE being the default I would say --append-cr=TRUE|FALSE with no default, meaning do not add the flag to the config at all. I though the idea was to append the CR by default, i.e. --append-cr=TRUE|FALSE with TRUE being the default. If you want to hardcode the default into the plugin, there is no benefit in using Bool over Flag, because Flag is actually a Bool with hardcoded default value. I actually started with a bool, default=True. I had the problem that the Default value was ignored, the value was None. Changing the default behavior is IMHO bad anyway does not matter if Bool or Flag. +1 Please advise what is you wish to be implemented :-) That depends. Is there a difference between do not set APPEND_CR ticket flag and set APPEND_CR ticket flag to false? For YubiKey hardware the flag is either present (true) or absent (false). This flag controls whether or not the carriage return is sent (present) or not (absent). Nathaniel -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] otptoken_yubikey, append CR by default and add a option for not doing so
Dne 9.4.2015 v 15:11 Luc de Louw napsal(a): On 04/09/2015 02:28 PM, Jan Cholasta wrote: Let's say you now introduce --no-cr flag. What if we decide to change the default to False? How would you then change the option/API? You would have to add --cr flag. That was the point - some clients would send ct flag, some no_cr and there would have to be special handling. It is more flexible IMO to just use something like --cr=TRUE|FALSE with TRUE being the default I would say --append-cr=TRUE|FALSE with no default, meaning do not add the flag to the config at all. I though the idea was to append the CR by default, i.e. --append-cr=TRUE|FALSE with TRUE being the default. If you want to hardcode the default into the plugin, there is no benefit in using Bool over Flag, because Flag is actually a Bool with hardcoded default value. I actually started with a bool, default=True. I had the problem that the Default value was ignored, the value was None. Changing the default behavior is IMHO bad anyway does not matter if Bool or Flag. +1 Please advise what is you wish to be implemented :-) That depends. Is there a difference between do not set APPEND_CR ticket flag and set APPEND_CR ticket flag to false? Thanks, Luc -- Jan Cholasta -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] otptoken_yubikey, append CR by default and add a option for not doing so
On 04/09/2015 02:28 PM, Jan Cholasta wrote: Let's say you now introduce --no-cr flag. What if we decide to change the default to False? How would you then change the option/API? You would have to add --cr flag. That was the point - some clients would send ct flag, some no_cr and there would have to be special handling. It is more flexible IMO to just use something like --cr=TRUE|FALSE with TRUE being the default I would say --append-cr=TRUE|FALSE with no default, meaning do not add the flag to the config at all. I though the idea was to append the CR by default, i.e. --append-cr=TRUE|FALSE with TRUE being the default. If you want to hardcode the default into the plugin, there is no benefit in using Bool over Flag, because Flag is actually a Bool with hardcoded default value. I actually started with a bool, default=True. I had the problem that the Default value was ignored, the value was None. Changing the default behavior is IMHO bad anyway does not matter if Bool or Flag. Please advise what is you wish to be implemented :-) Thanks, Luc -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] otptoken_yubikey, append CR by default and add a option for not doing so
Dne 9.4.2015 v 12:42 Martin Kosek napsal(a): On 04/09/2015 12:30 PM, Jan Cholasta wrote: Dne 8.4.2015 v 22:52 Martin Kosek napsal(a): On 04/08/2015 06:03 PM, Nathaniel McCallum wrote: On Wed, 2015-04-08 at 17:53 +0200, Martin Basti wrote: On 08/04/15 17:46, Luc de Louw wrote: On 04/08/2015 05:14 PM, Martin Basti wrote: On 08/04/15 17:12, Luc de Louw wrote: On 04/08/2015 05:05 PM, Martin Basti wrote: On 08/04/15 16:55, Nathaniel McCallum wrote: On Wed, 2015-04-08 at 11:57 +0200, Luc de Louw wrote: Hi there, At the moment ipa otptoken-add-yubikey does not add the parameter APPEND_CR. This prevents submit the password+OTP. APPEND_CR is usually very handy, most people use this functionality. The patch changes the behavior to set APPEND_CR by default and let the user override this by using the the --do-not-append-cr option. This patch is very helpful and I would like to see it merged. Thanks Luc! 1. This patch needs to be formatted according to the FreeIPA formatting. See: https://www.freeipa.org/page/Contribute/Patch_Format 2. The flag should be named no_cr instead of do_not_append_cr. 3. The comment is not necessary since what the code does is obvious. Nathaniel Hello, 4) this patch changes API, so please run ./makeapi to regenerate API.txt file and add changes into patch + please bum API minor version in VERSION file thanks. Hi, When running makeaip, I get the following error: File /home/luc/freeipa/ipalib/constants.py, line 25, in module from ipaplatform.paths import paths ImportError: No module named paths Any hints? The other changes are ready to submit. Thanks, Luc You may need to run 'make version-upgrade' or 'make' to prepare the module. If it will not work, you can send incomplete patch, I will add API changes there, just bump VERSION please Martin, Thanks for your hints, seems to work, please have a look at it... Thanks, Luc Thanks, please change the comment too -IPA_API_VERSION_MINOR=116 +IPA_API_VERSION_MINOR=117 # Last change: tbordaz - Add stageuser_add command Otherwise patch looks good, but Nathaniel is the OTP guru, he should say final ack. I'm also a tough reviewer. :) 1. Remove the unnecessary code comment. 2. There appears to be inconsistent indentation in the flag parameter specification. It is probably a mix of tabs and spaces. 3. The git commit comment should contain one short summary line without terminating punctuation followed by any necessary explanatory paragraphs. You can change this via the --amend option to git commit. Try the following: Enable YubiKey carriage return emission via otptoken-add-yubikey Before this patch, YubiKeys programmed by IPA would not emit the carriage return character at the end of the OTP value. This requires the user to press his YubiKey and then (unnecessarily) the Enter or Return key. After this patch, the user only needs to press the YubiKey. Should a user desire to omit the carriage return character, the --no- cr option can be specified. Nathaniel One more note to the API. By my experience, using a Flag for a boolean data input has often proved to be a bad call. Let's say you now introduce --no-cr flag. What if we decide to change the default to False? How would you then change the option/API? You would have to add --cr flag. That was the point - some clients would send ct flag, some no_cr and there would have to be special handling. It is more flexible IMO to just use something like --cr=TRUE|FALSE with TRUE being the default I would say --append-cr=TRUE|FALSE with no default, meaning do not add the flag to the config at all. I though the idea was to append the CR by default, i.e. --append-cr=TRUE|FALSE with TRUE being the default. If you want to hardcode the default into the plugin, there is no benefit in using Bool over Flag, because Flag is actually a Bool with hardcoded default value. -- Jan Cholasta -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] otptoken_yubikey, append CR by default and add a option for not doing so
Dne 8.4.2015 v 22:52 Martin Kosek napsal(a): On 04/08/2015 06:03 PM, Nathaniel McCallum wrote: On Wed, 2015-04-08 at 17:53 +0200, Martin Basti wrote: On 08/04/15 17:46, Luc de Louw wrote: On 04/08/2015 05:14 PM, Martin Basti wrote: On 08/04/15 17:12, Luc de Louw wrote: On 04/08/2015 05:05 PM, Martin Basti wrote: On 08/04/15 16:55, Nathaniel McCallum wrote: On Wed, 2015-04-08 at 11:57 +0200, Luc de Louw wrote: Hi there, At the moment ipa otptoken-add-yubikey does not add the parameter APPEND_CR. This prevents submit the password+OTP. APPEND_CR is usually very handy, most people use this functionality. The patch changes the behavior to set APPEND_CR by default and let the user override this by using the the --do-not-append-cr option. This patch is very helpful and I would like to see it merged. Thanks Luc! 1. This patch needs to be formatted according to the FreeIPA formatting. See: https://www.freeipa.org/page/Contribute/Patch_Format 2. The flag should be named no_cr instead of do_not_append_cr. 3. The comment is not necessary since what the code does is obvious. Nathaniel Hello, 4) this patch changes API, so please run ./makeapi to regenerate API.txt file and add changes into patch + please bum API minor version in VERSION file thanks. Hi, When running makeaip, I get the following error: File /home/luc/freeipa/ipalib/constants.py, line 25, in module from ipaplatform.paths import paths ImportError: No module named paths Any hints? The other changes are ready to submit. Thanks, Luc You may need to run 'make version-upgrade' or 'make' to prepare the module. If it will not work, you can send incomplete patch, I will add API changes there, just bump VERSION please Martin, Thanks for your hints, seems to work, please have a look at it... Thanks, Luc Thanks, please change the comment too -IPA_API_VERSION_MINOR=116 +IPA_API_VERSION_MINOR=117 # Last change: tbordaz - Add stageuser_add command Otherwise patch looks good, but Nathaniel is the OTP guru, he should say final ack. I'm also a tough reviewer. :) 1. Remove the unnecessary code comment. 2. There appears to be inconsistent indentation in the flag parameter specification. It is probably a mix of tabs and spaces. 3. The git commit comment should contain one short summary line without terminating punctuation followed by any necessary explanatory paragraphs. You can change this via the --amend option to git commit. Try the following: Enable YubiKey carriage return emission via otptoken-add-yubikey Before this patch, YubiKeys programmed by IPA would not emit the carriage return character at the end of the OTP value. This requires the user to press his YubiKey and then (unnecessarily) the Enter or Return key. After this patch, the user only needs to press the YubiKey. Should a user desire to omit the carriage return character, the --no- cr option can be specified. Nathaniel One more note to the API. By my experience, using a Flag for a boolean data input has often proved to be a bad call. Let's say you now introduce --no-cr flag. What if we decide to change the default to False? How would you then change the option/API? You would have to add --cr flag. It is more flexible IMO to just use something like --cr=TRUE|FALSE with TRUE being the default I would say --append-cr=TRUE|FALSE with no default, meaning do not add the flag to the config at all. Martin -- Jan Cholasta -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] otptoken_yubikey, append CR by default and add a option for not doing so
On 04/09/2015 12:30 PM, Jan Cholasta wrote: Dne 8.4.2015 v 22:52 Martin Kosek napsal(a): On 04/08/2015 06:03 PM, Nathaniel McCallum wrote: On Wed, 2015-04-08 at 17:53 +0200, Martin Basti wrote: On 08/04/15 17:46, Luc de Louw wrote: On 04/08/2015 05:14 PM, Martin Basti wrote: On 08/04/15 17:12, Luc de Louw wrote: On 04/08/2015 05:05 PM, Martin Basti wrote: On 08/04/15 16:55, Nathaniel McCallum wrote: On Wed, 2015-04-08 at 11:57 +0200, Luc de Louw wrote: Hi there, At the moment ipa otptoken-add-yubikey does not add the parameter APPEND_CR. This prevents submit the password+OTP. APPEND_CR is usually very handy, most people use this functionality. The patch changes the behavior to set APPEND_CR by default and let the user override this by using the the --do-not-append-cr option. This patch is very helpful and I would like to see it merged. Thanks Luc! 1. This patch needs to be formatted according to the FreeIPA formatting. See: https://www.freeipa.org/page/Contribute/Patch_Format 2. The flag should be named no_cr instead of do_not_append_cr. 3. The comment is not necessary since what the code does is obvious. Nathaniel Hello, 4) this patch changes API, so please run ./makeapi to regenerate API.txt file and add changes into patch + please bum API minor version in VERSION file thanks. Hi, When running makeaip, I get the following error: File /home/luc/freeipa/ipalib/constants.py, line 25, in module from ipaplatform.paths import paths ImportError: No module named paths Any hints? The other changes are ready to submit. Thanks, Luc You may need to run 'make version-upgrade' or 'make' to prepare the module. If it will not work, you can send incomplete patch, I will add API changes there, just bump VERSION please Martin, Thanks for your hints, seems to work, please have a look at it... Thanks, Luc Thanks, please change the comment too -IPA_API_VERSION_MINOR=116 +IPA_API_VERSION_MINOR=117 # Last change: tbordaz - Add stageuser_add command Otherwise patch looks good, but Nathaniel is the OTP guru, he should say final ack. I'm also a tough reviewer. :) 1. Remove the unnecessary code comment. 2. There appears to be inconsistent indentation in the flag parameter specification. It is probably a mix of tabs and spaces. 3. The git commit comment should contain one short summary line without terminating punctuation followed by any necessary explanatory paragraphs. You can change this via the --amend option to git commit. Try the following: Enable YubiKey carriage return emission via otptoken-add-yubikey Before this patch, YubiKeys programmed by IPA would not emit the carriage return character at the end of the OTP value. This requires the user to press his YubiKey and then (unnecessarily) the Enter or Return key. After this patch, the user only needs to press the YubiKey. Should a user desire to omit the carriage return character, the --no- cr option can be specified. Nathaniel One more note to the API. By my experience, using a Flag for a boolean data input has often proved to be a bad call. Let's say you now introduce --no-cr flag. What if we decide to change the default to False? How would you then change the option/API? You would have to add --cr flag. That was the point - some clients would send ct flag, some no_cr and there would have to be special handling. It is more flexible IMO to just use something like --cr=TRUE|FALSE with TRUE being the default I would say --append-cr=TRUE|FALSE with no default, meaning do not add the flag to the config at all. I though the idea was to append the CR by default, i.e. --append-cr=TRUE|FALSE with TRUE being the default. -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] otptoken_yubikey, append CR by default and add a option for not doing so
On 04/08/2015 06:03 PM, Nathaniel McCallum wrote: On Wed, 2015-04-08 at 17:53 +0200, Martin Basti wrote: On 08/04/15 17:46, Luc de Louw wrote: On 04/08/2015 05:14 PM, Martin Basti wrote: On 08/04/15 17:12, Luc de Louw wrote: On 04/08/2015 05:05 PM, Martin Basti wrote: On 08/04/15 16:55, Nathaniel McCallum wrote: On Wed, 2015-04-08 at 11:57 +0200, Luc de Louw wrote: Hi there, At the moment ipa otptoken-add-yubikey does not add the parameter APPEND_CR. This prevents submit the password+OTP. APPEND_CR is usually very handy, most people use this functionality. The patch changes the behavior to set APPEND_CR by default and let the user override this by using the the --do-not-append-cr option. This patch is very helpful and I would like to see it merged. Thanks Luc! 1. This patch needs to be formatted according to the FreeIPA formatting. See: https://www.freeipa.org/page/Contribute/Patch_Format 2. The flag should be named no_cr instead of do_not_append_cr. 3. The comment is not necessary since what the code does is obvious. Nathaniel Hello, 4) this patch changes API, so please run ./makeapi to regenerate API.txt file and add changes into patch + please bum API minor version in VERSION file thanks. Hi, When running makeaip, I get the following error: File /home/luc/freeipa/ipalib/constants.py, line 25, in module from ipaplatform.paths import paths ImportError: No module named paths Any hints? The other changes are ready to submit. Thanks, Luc You may need to run 'make version-upgrade' or 'make' to prepare the module. If it will not work, you can send incomplete patch, I will add API changes there, just bump VERSION please Martin, Thanks for your hints, seems to work, please have a look at it... Thanks, Luc Thanks, please change the comment too -IPA_API_VERSION_MINOR=116 +IPA_API_VERSION_MINOR=117 # Last change: tbordaz - Add stageuser_add command Otherwise patch looks good, but Nathaniel is the OTP guru, he should say final ack. I'm also a tough reviewer. :) 1. Remove the unnecessary code comment. 2. There appears to be inconsistent indentation in the flag parameter specification. It is probably a mix of tabs and spaces. 3. The git commit comment should contain one short summary line without terminating punctuation followed by any necessary explanatory paragraphs. You can change this via the --amend option to git commit. Try the following: Enable YubiKey carriage return emission via otptoken-add-yubikey Before this patch, YubiKeys programmed by IPA would not emit the carriage return character at the end of the OTP value. This requires the user to press his YubiKey and then (unnecessarily) the Enter or Return key. After this patch, the user only needs to press the YubiKey. Should a user desire to omit the carriage return character, the --no- cr option can be specified. Nathaniel One more note to the API. By my experience, using a Flag for a boolean data input has often proved to be a bad call. Let's say you now introduce --no-cr flag. What if we decide to change the default to False? How would you then change the option/API? It is more flexible IMO to just use something like --cr=TRUE|FALSE with TRUE being the default Martin -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] otptoken_yubikey, append CR by default and add a option for not doing so
On 08/04/15 17:46, Luc de Louw wrote: On 04/08/2015 05:14 PM, Martin Basti wrote: On 08/04/15 17:12, Luc de Louw wrote: On 04/08/2015 05:05 PM, Martin Basti wrote: On 08/04/15 16:55, Nathaniel McCallum wrote: On Wed, 2015-04-08 at 11:57 +0200, Luc de Louw wrote: Hi there, At the moment ipa otptoken-add-yubikey does not add the parameter APPEND_CR. This prevents submit the password+OTP. APPEND_CR is usually very handy, most people use this functionality. The patch changes the behavior to set APPEND_CR by default and let the user override this by using the the --do-not-append-cr option. This patch is very helpful and I would like to see it merged. Thanks Luc! 1. This patch needs to be formatted according to the FreeIPA formatting. See: https://www.freeipa.org/page/Contribute/Patch_Format 2. The flag should be named no_cr instead of do_not_append_cr. 3. The comment is not necessary since what the code does is obvious. Nathaniel Hello, 4) this patch changes API, so please run ./makeapi to regenerate API.txt file and add changes into patch + please bum API minor version in VERSION file thanks. Hi, When running makeaip, I get the following error: File /home/luc/freeipa/ipalib/constants.py, line 25, in module from ipaplatform.paths import paths ImportError: No module named paths Any hints? The other changes are ready to submit. Thanks, Luc You may need to run 'make version-upgrade' or 'make' to prepare the module. If it will not work, you can send incomplete patch, I will add API changes there, just bump VERSION please Martin, Thanks for your hints, seems to work, please have a look at it... Thanks, Luc Thanks, please change the comment too -IPA_API_VERSION_MINOR=116 +IPA_API_VERSION_MINOR=117 # Last change: tbordaz - Add stageuser_add command Otherwise patch looks good, but Nathaniel is the OTP guru, he should say final ack. -- Martin Basti -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] otptoken_yubikey, append CR by default and add a option for not doing so
On 04/08/2015 05:14 PM, Martin Basti wrote:
On 08/04/15 17:12, Luc de Louw wrote:
On 04/08/2015 05:05 PM, Martin Basti wrote:
On 08/04/15 16:55, Nathaniel McCallum wrote:
On Wed, 2015-04-08 at 11:57 +0200, Luc de Louw wrote:
Hi there,
At the moment ipa otptoken-add-yubikey does not add the parameter
APPEND_CR. This prevents submit the password+OTP. APPEND_CR is
usually
very handy, most people use this functionality.
The patch changes the behavior to set APPEND_CR by default and let
the
user override this by using the the --do-not-append-cr option.
This patch is very helpful and I would like to see it merged. Thanks
Luc!
1. This patch needs to be formatted according to the FreeIPA
formatting. See: https://www.freeipa.org/page/Contribute/Patch_Format
2. The flag should be named no_cr instead of do_not_append_cr.
3. The comment is not necessary since what the code does is obvious.
Nathaniel
Hello,
4) this patch changes API, so please run ./makeapi to regenerate API.txt
file and add changes into patch + please bum API minor version in
VERSION file
thanks.
Hi,
When running makeaip, I get the following error:
File /home/luc/freeipa/ipalib/constants.py, line 25, in module
from ipaplatform.paths import paths
ImportError: No module named paths
Any hints?
The other changes are ready to submit.
Thanks,
Luc
You may need to run 'make version-upgrade' or 'make' to prepare the module.
If it will not work, you can send incomplete patch, I will add API
changes there, just bump VERSION please
Martin,
Thanks for your hints, seems to work, please have a look at it...
Thanks,
Luc
From f1a01a7f984e60df6978a506af94ea9e1d8098ea Mon Sep 17 00:00:00 2001
From: Luc de Louw ldel...@redhat.com
Date: Wed, 8 Apr 2015 17:41:33 +0200
Subject: [PATCH] At the moment ipa otptoken-add-yubikey does not add the
parameter APPEND_CR. This prevents submit the password+OTP. APPEND_CR is
usually very handy, most people use this functionality.
The patch changes the behavior to set APPEND_CR by default and let
the user override this by using the the --no-cr option.
---
API.txt| 3 ++-
VERSION| 2 +-
ipalib/plugins/otptoken_yubikey.py | 13 -
3 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/API.txt b/API.txt
index f747765d7f9c87761fed0277cd59d1bc3fbd57e9..d2c57e48e775dacdfe70fbd1eae6249f1c82e1ad 100644
--- a/API.txt
+++ b/API.txt
@@ -2661,7 +2661,7 @@ output: Output('completed', type 'int', None)
output: Output('failed', type 'dict', None)
output: Entry('result', type 'dict', Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
command: otptoken_add_yubikey
-args: 1,8,1
+args: 1,9,1
arg: Str('ipatokenuniqueid?', cli_name='id', primary_key=True)
option: Str('description?', cli_name='desc')
option: Bool('ipatokendisabled?', cli_name='disabled')
@@ -2669,6 +2669,7 @@ option: DateTime('ipatokennotafter?', cli_name='not_after')
option: DateTime('ipatokennotbefore?', cli_name='not_before')
option: IntEnum('ipatokenotpdigits?', autofill=True, cli_name='digits', default=6, values=(6, 8))
option: Str('ipatokenowner?', cli_name='owner')
+option: Flag('no_cr?', autofill=True, cli_name='no_cr', default=False, required=False)
option: IntEnum('slot?', cli_name='slot', values=(1, 2))
option: Str('version?', exclude='webui')
output: Output('result', None, None)
diff --git a/VERSION b/VERSION
index b584eb4584ea45881e5329a846dae0df7e231844..5acbdf1caa287d664df00260620809d06f5e5eb1 100644
--- a/VERSION
+++ b/VERSION
@@ -90,5 +90,5 @@ IPA_DATA_VERSION=2010061412
# #
IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=116
+IPA_API_VERSION_MINOR=117
# Last change: tbordaz - Add stageuser_add command
diff --git a/ipalib/plugins/otptoken_yubikey.py b/ipalib/plugins/otptoken_yubikey.py
index 58fc18308f0cfe407881b5fbb5e653c5afbd0eba..a48a1294a6349a1721601e2100088a0a458c5e84 100644
--- a/ipalib/plugins/otptoken_yubikey.py
+++ b/ipalib/plugins/otptoken_yubikey.py
@@ -17,7 +17,7 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see http://www.gnu.org/licenses/.
-from ipalib import _, Str, IntEnum
+from ipalib import _, Str, IntEnum, Flag
from ipalib.errors import NotFound
from ipalib.plugable import Registry
from ipalib.frontend import Command
@@ -65,6 +65,12 @@ class otptoken_add_yubikey(Command):
label=_('YubiKey slot'),
values=(1, 2),
),
+ Flag('no_cr?',
+ cli_name='no_cr',
+ label=_('Do not append a CR after sending the OTP (default: false)'),
+ default=False,
+ required=False,
+ ),
) + tuple(x for x in otptoken.takes_params if x.name in (
'description',
'ipatokenowner',
@@ -107,6 +113,11 @@ class otptoken_add_yubikey(Command):
Re: [Freeipa-devel] [PATCH] otptoken_yubikey, append CR by default and add a option for not doing so
On 04/08/2015 05:53 PM, Martin Basti wrote: On 08/04/15 17:46, Luc de Louw wrote: On 04/08/2015 05:14 PM, Martin Basti wrote: On 08/04/15 17:12, Luc de Louw wrote: On 04/08/2015 05:05 PM, Martin Basti wrote: On 08/04/15 16:55, Nathaniel McCallum wrote: On Wed, 2015-04-08 at 11:57 +0200, Luc de Louw wrote: Hi there, At the moment ipa otptoken-add-yubikey does not add the parameter APPEND_CR. This prevents submit the password+OTP. APPEND_CR is usually very handy, most people use this functionality. The patch changes the behavior to set APPEND_CR by default and let the user override this by using the the --do-not-append-cr option. This patch is very helpful and I would like to see it merged. Thanks Luc! 1. This patch needs to be formatted according to the FreeIPA formatting. See: https://www.freeipa.org/page/Contribute/Patch_Format 2. The flag should be named no_cr instead of do_not_append_cr. 3. The comment is not necessary since what the code does is obvious. Nathaniel Hello, 4) this patch changes API, so please run ./makeapi to regenerate API.txt file and add changes into patch + please bum API minor version in VERSION file thanks. Hi, When running makeaip, I get the following error: File /home/luc/freeipa/ipalib/constants.py, line 25, in module from ipaplatform.paths import paths ImportError: No module named paths Any hints? The other changes are ready to submit. Thanks, Luc You may need to run 'make version-upgrade' or 'make' to prepare the module. If it will not work, you can send incomplete patch, I will add API changes there, just bump VERSION please Martin, Thanks for your hints, seems to work, please have a look at it... Thanks, Luc Thanks, please change the comment too -IPA_API_VERSION_MINOR=116 +IPA_API_VERSION_MINOR=117 # Last change: tbordaz - Add stageuser_add command Otherwise patch looks good, but Nathaniel is the OTP guru, he should say final ack. Here we are Thanks, Luc From 064436ea7d111c89147213b319ccb76b6090a650 Mon Sep 17 00:00:00 2001 From: Luc de Louw ldel...@redhat.com Date: Wed, 8 Apr 2015 18:01:38 +0200 Subject: [PATCH] Added last change statement --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 5acbdf1caa287d664df00260620809d06f5e5eb1..54c29d99f9ef03ac69e55da09f495082e38207ce 100644 --- a/VERSION +++ b/VERSION @@ -91,4 +91,4 @@ IPA_DATA_VERSION=2010061412 IPA_API_VERSION_MAJOR=2 IPA_API_VERSION_MINOR=117 -# Last change: tbordaz - Add stageuser_add command +# Last change: ldelouw - Add no-cr option to otptoken-add-yubikey -- 2.1.0 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] otptoken_yubikey, append CR by default and add a option for not doing so
On Wed, 2015-04-08 at 17:53 +0200, Martin Basti wrote: On 08/04/15 17:46, Luc de Louw wrote: On 04/08/2015 05:14 PM, Martin Basti wrote: On 08/04/15 17:12, Luc de Louw wrote: On 04/08/2015 05:05 PM, Martin Basti wrote: On 08/04/15 16:55, Nathaniel McCallum wrote: On Wed, 2015-04-08 at 11:57 +0200, Luc de Louw wrote: Hi there, At the moment ipa otptoken-add-yubikey does not add the parameter APPEND_CR. This prevents submit the password+OTP. APPEND_CR is usually very handy, most people use this functionality. The patch changes the behavior to set APPEND_CR by default and let the user override this by using the the --do-not-append-cr option. This patch is very helpful and I would like to see it merged. Thanks Luc! 1. This patch needs to be formatted according to the FreeIPA formatting. See: https://www.freeipa.org/page/Contribute/Patch_Format 2. The flag should be named no_cr instead of do_not_append_cr. 3. The comment is not necessary since what the code does is obvious. Nathaniel Hello, 4) this patch changes API, so please run ./makeapi to regenerate API.txt file and add changes into patch + please bum API minor version in VERSION file thanks. Hi, When running makeaip, I get the following error: File /home/luc/freeipa/ipalib/constants.py, line 25, in module from ipaplatform.paths import paths ImportError: No module named paths Any hints? The other changes are ready to submit. Thanks, Luc You may need to run 'make version-upgrade' or 'make' to prepare the module. If it will not work, you can send incomplete patch, I will add API changes there, just bump VERSION please Martin, Thanks for your hints, seems to work, please have a look at it... Thanks, Luc Thanks, please change the comment too -IPA_API_VERSION_MINOR=116 +IPA_API_VERSION_MINOR=117 # Last change: tbordaz - Add stageuser_add command Otherwise patch looks good, but Nathaniel is the OTP guru, he should say final ack. I'm also a tough reviewer. :) 1. Remove the unnecessary code comment. 2. There appears to be inconsistent indentation in the flag parameter specification. It is probably a mix of tabs and spaces. 3. The git commit comment should contain one short summary line without terminating punctuation followed by any necessary explanatory paragraphs. You can change this via the --amend option to git commit. Try the following: Enable YubiKey carriage return emission via otptoken-add-yubikey Before this patch, YubiKeys programmed by IPA would not emit the carriage return character at the end of the OTP value. This requires the user to press his YubiKey and then (unnecessarily) the Enter or Return key. After this patch, the user only needs to press the YubiKey. Should a user desire to omit the carriage return character, the --no- cr option can be specified. Nathaniel -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] otptoken_yubikey, append CR by default and add a option for not doing so
On 04/08/2015 05:05 PM, Martin Basti wrote: On 08/04/15 16:55, Nathaniel McCallum wrote: On Wed, 2015-04-08 at 11:57 +0200, Luc de Louw wrote: Hi there, At the moment ipa otptoken-add-yubikey does not add the parameter APPEND_CR. This prevents submit the password+OTP. APPEND_CR is usually very handy, most people use this functionality. The patch changes the behavior to set APPEND_CR by default and let the user override this by using the the --do-not-append-cr option. This patch is very helpful and I would like to see it merged. Thanks Luc! 1. This patch needs to be formatted according to the FreeIPA formatting. See: https://www.freeipa.org/page/Contribute/Patch_Format 2. The flag should be named no_cr instead of do_not_append_cr. 3. The comment is not necessary since what the code does is obvious. Nathaniel Hello, 4) this patch changes API, so please run ./makeapi to regenerate API.txt file and add changes into patch + please bum API minor version in VERSION file thanks. Hi, When running makeaip, I get the following error: File /home/luc/freeipa/ipalib/constants.py, line 25, in module from ipaplatform.paths import paths ImportError: No module named paths Any hints? The other changes are ready to submit. Thanks, Luc -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] otptoken_yubikey, append CR by default and add a option for not doing so
On Wed, 2015-04-08 at 11:57 +0200, Luc de Louw wrote: Hi there, At the moment ipa otptoken-add-yubikey does not add the parameter APPEND_CR. This prevents submit the password+OTP. APPEND_CR is usually very handy, most people use this functionality. The patch changes the behavior to set APPEND_CR by default and let the user override this by using the the --do-not-append-cr option. This patch is very helpful and I would like to see it merged. Thanks Luc! 1. This patch needs to be formatted according to the FreeIPA formatting. See: https://www.freeipa.org/page/Contribute/Patch_Format 2. The flag should be named no_cr instead of do_not_append_cr. 3. The comment is not necessary since what the code does is obvious. Nathaniel -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] otptoken_yubikey, append CR by default and add a option for not doing so
On 08/04/15 16:55, Nathaniel McCallum wrote: On Wed, 2015-04-08 at 11:57 +0200, Luc de Louw wrote: Hi there, At the moment ipa otptoken-add-yubikey does not add the parameter APPEND_CR. This prevents submit the password+OTP. APPEND_CR is usually very handy, most people use this functionality. The patch changes the behavior to set APPEND_CR by default and let the user override this by using the the --do-not-append-cr option. This patch is very helpful and I would like to see it merged. Thanks Luc! 1. This patch needs to be formatted according to the FreeIPA formatting. See: https://www.freeipa.org/page/Contribute/Patch_Format 2. The flag should be named no_cr instead of do_not_append_cr. 3. The comment is not necessary since what the code does is obvious. Nathaniel Hello, 4) this patch changes API, so please run ./makeapi to regenerate API.txt file and add changes into patch + please bum API minor version in VERSION file thanks. -- Martin Basti -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] otptoken_yubikey, append CR by default and add a option for not doing so
On 08/04/15 17:12, Luc de Louw wrote: On 04/08/2015 05:05 PM, Martin Basti wrote: On 08/04/15 16:55, Nathaniel McCallum wrote: On Wed, 2015-04-08 at 11:57 +0200, Luc de Louw wrote: Hi there, At the moment ipa otptoken-add-yubikey does not add the parameter APPEND_CR. This prevents submit the password+OTP. APPEND_CR is usually very handy, most people use this functionality. The patch changes the behavior to set APPEND_CR by default and let the user override this by using the the --do-not-append-cr option. This patch is very helpful and I would like to see it merged. Thanks Luc! 1. This patch needs to be formatted according to the FreeIPA formatting. See: https://www.freeipa.org/page/Contribute/Patch_Format 2. The flag should be named no_cr instead of do_not_append_cr. 3. The comment is not necessary since what the code does is obvious. Nathaniel Hello, 4) this patch changes API, so please run ./makeapi to regenerate API.txt file and add changes into patch + please bum API minor version in VERSION file thanks. Hi, When running makeaip, I get the following error: File /home/luc/freeipa/ipalib/constants.py, line 25, in module from ipaplatform.paths import paths ImportError: No module named paths Any hints? The other changes are ready to submit. Thanks, Luc You may need to run 'make version-upgrade' or 'make' to prepare the module. If it will not work, you can send incomplete patch, I will add API changes there, just bump VERSION please -- Martin Basti -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
