Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On 12/04/2014 07:15 PM, Nathaniel McCallum wrote: On Thu, 2014-12-04 at 14:56 +0100, Petr Vobornik wrote: On 2.12.2014 20:57, Nathaniel McCallum wrote: Works fine. python part of 0004: ACK, but VERSION needs to be updated before push 0005: ACK Fixed and rebased. Patch numbers have changed: 0004 = 0001 0005 = 0002 One question before push: For per-token configuration, do you intent to extend each token, regardless of type, by 'ipatokenOTPConfig' object class? I.e. to have config attributes for both types? Or do you plan to have special object classes for each token type as we now have for tokens? I would probably just add the TOTP options to the ipatokenTOTP object class as MAY. Same for HOTP. The attributes were designed to look like the other token-type-specific attributes. I think we are just waiting on Thierry's review of the C code. :) Thierry already wrote: regarding the DS plugin part of 0004, the patch is good to me. For the ipa plugins part I am too novice. Therefore: 0001 Pushed to: master: 9baa93da1cbf56c2a6f7e82e099bc3ff3f19e2e4 ipa-4-1: 3013385ca4a28a4f203fae6dbef34321720d8879 0002 Pushed to: ipa-4-1: f5ae902eb5c391bd6150c99d5b3316be937aa459 master: b01767c69d69806b3c701242d617b6fa08e7d882 Nathaniel -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On 12/05/2014 01:46 PM, Petr Vobornik wrote: On 12/04/2014 07:15 PM, Nathaniel McCallum wrote: On Thu, 2014-12-04 at 14:56 +0100, Petr Vobornik wrote: On 2.12.2014 20:57, Nathaniel McCallum wrote: Works fine. python part of 0004: ACK, but VERSION needs to be updated before push 0005: ACK Fixed and rebased. Patch numbers have changed: 0004 = 0001 0005 = 0002 One question before push: For per-token configuration, do you intent to extend each token, regardless of type, by 'ipatokenOTPConfig' object class? I.e. to have config attributes for both types? Or do you plan to have special object classes for each token type as we now have for tokens? I would probably just add the TOTP options to the ipatokenTOTP object class as MAY. Same for HOTP. The attributes were designed to look like the other token-type-specific attributes. I think we are just waiting on Thierry's review of the C code. :) Thierry already wrote: regarding the DS plugin part of 0004, the patch is good to me. For the ipa plugins part I am too novice. Therefore: 0001 Pushed to: master: 9baa93da1cbf56c2a6f7e82e099bc3ff3f19e2e4 ipa-4-1: 3013385ca4a28a4f203fae6dbef34321720d8879 0002 Pushed to: ipa-4-1: f5ae902eb5c391bd6150c99d5b3316be937aa459 master: b01767c69d69806b3c701242d617b6fa08e7d882 Thanks to all for resolving this RFE and this thread. It started to be little bit tangled :-) ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On 2.12.2014 20:57, Nathaniel McCallum wrote: I'm little confused with a state of reviews. Thierry were some of the patches ACKed in different threads or are they under review (I'm not reviewing DS plugin parts)? Patches 0001, 0002, 0003 are ACKed by Thierry, but not merged. They can and should be merged as they fix an independent bug. Ah, I meant adding the token config to cn=otp,SUFFIX directly, but if we want to make TOTP/HOTP token config as separate entries (to enable future per-token overrides), your approach should make sense. Rather adding Rob to CC for sanity. That would work too. I'm open to that. I am just not sure we should create them as separate plugins, I think the new commands should be rather added to otp plugin directly so that they show in ipa help otptoken instead of adding 2 new topics just for OTP config. I can play with that. Do you plan to change it? I like the idea of a single point of help for OTP but I'm also unsure about the length of the commands. Current solution is also more consistent with a rest of the framework. Would it be something like: otptoken-totpconfig-(show|mod) otptoken-hotpconfig-(show|mod) In the latest patch, I merged totpconfig-* and hotpconfig-* into a single otpconfig-* plugin. Maybe it would be better to introduce more help topics for otp. This concept is used for HBAC already: $ ipa help hbac hbacsvcgroup HBAC Service Groups hbacsvc HBAC Services hbacrule Host-based access control $ ipa help hbacrule Host-based access control ... a lot of text So we could introduce otp umbrella topic: $ ipa help otp opttoken OTP tokens' totpconfig TOTP configuration options hotpconfig HOTP configuration options I added a fifth patch (0005) which creates an otp umbrella topic. We can merge it or not. Nathaniel No worries ATM, you can wait for proper review. I was just looking at the new API to make sure we are on the same page - we seem to mostly are. Martin Commenting just patch 0004: 1. Requires rebase because of API change. Fixed. 2. git diff HEAD~4 -U0 | pep8 --diff I would ignore E124 and fix E302 (5x) Fixed. I did not test actual functionality yet. The attached patches I think have a much better overall aesthetic. Now patch 0004 introduces only two new commands: * otpconfig-mod * otpconfig-show Under the covers, a single configuration entity is used: * cn=otp,cn=etc,$SUFFIX Other than these small changes, there are no changes to patch 0004. I have not tested the latest changes, however, due to an unrelated build issue I'm working on. Patch 0005 introduces an umbrella help topic for all OTP related commands (currently: otpconfig, otptoken, otptoken-yubikey). Nathaniel Works fine. python part of 0004: ACK, but VERSION needs to be updated before push 0005: ACK One question before push: For per-token configuration, do you intent to extend each token, regardless of type, by 'ipatokenOTPConfig' object class? I.e. to have config attributes for both types? Or do you plan to have special object classes for each token type as we now have for tokens? -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On Thu, 2014-12-04 at 14:56 +0100, Petr Vobornik wrote:
On 2.12.2014 20:57, Nathaniel McCallum wrote:
The attached patches I think have a much better overall aesthetic. Now
patch 0004 introduces only two new commands:
* otpconfig-mod
* otpconfig-show
Under the covers, a single configuration entity is used:
* cn=otp,cn=etc,$SUFFIX
Other than these small changes, there are no changes to patch 0004. I
have not tested the latest changes, however, due to an unrelated build
issue I'm working on.
Patch 0005 introduces an umbrella help topic for all OTP related
commands (currently: otpconfig, otptoken, otptoken-yubikey).
Nathaniel
Works fine.
python part of 0004: ACK, but VERSION needs to be updated before push
0005: ACK
Fixed and rebased. Patch numbers have changed:
0004 = 0001
0005 = 0002
One question before push: For per-token configuration, do you intent to
extend each token, regardless of type, by 'ipatokenOTPConfig' object
class? I.e. to have config attributes for both types? Or do you plan to
have special object classes for each token type as we now have for tokens?
I would probably just add the TOTP options to the ipatokenTOTP object
class as MAY. Same for HOTP. The attributes were designed to look like
the other token-type-specific attributes.
I think we are just waiting on Thierry's review of the C code. :)
Nathaniel
From 4be7cd92c19cee4ca8861a520fa490201864ae6a Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum npmccal...@redhat.com
Date: Tue, 11 Nov 2014 14:41:42 -0500
Subject: [PATCH 1/2] Make token auth and sync windows configurable
This introduces two new CLI commands:
* otpconfig-show
* otpconfig-mod
https://fedorahosted.org/freeipa/ticket/4511
---
ACI.txt | 2 +
API.txt | 25
VERSION | 4 +-
daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c | 77
daemons/ipa-slapi-plugins/ipa-pwd-extop/syncreq.c | 4 +-
daemons/ipa-slapi-plugins/libotp/otp_config.c | 89 +-
daemons/ipa-slapi-plugins/libotp/otp_config.h | 17 +++
daemons/ipa-slapi-plugins/libotp/otp_token.c | 139 +-
daemons/ipa-slapi-plugins/libotp/otp_token.h | 26 ++--
install/share/70ipaotp.ldif | 5 +
install/updates/40-otp.update | 9 ++
ipalib/plugins/otpconfig.py | 119 ++
12 files changed, 362 insertions(+), 154 deletions(-)
create mode 100644 ipalib/plugins/otpconfig.py
diff --git a/ACI.txt b/ACI.txt
index 6680f658ee1aa0f961b2681f700557ce6b9238f8..e4b4032d4e021bed6ade8a6cb66e39621bedfb85 100644
--- a/ACI.txt
+++ b/ACI.txt
@@ -154,6 +154,8 @@ dn: cn=ng,cn=alt,dc=ipa,dc=example
aci: (targetattr = cn || createtimestamp || description || entryusn || hostcategory || ipaenabledflag || ipauniqueid || modifytimestamp || nisdomainname || objectclass || usercategory)(targetfilter = (objectclass=ipanisnetgroup))(version 3.0;acl permission:System: Read Netgroups;allow (compare,read,search) userdn = ldap:///all;;)
dn: cn=ng,cn=alt,dc=ipa,dc=example
aci: (targetfilter = (objectclass=ipanisnetgroup))(version 3.0;acl permission:System: Remove Netgroups;allow (delete) groupdn = ldap:///cn=System: Remove Netgroups,cn=permissions,cn=pbac,dc=ipa,dc=example;)
+dn: cn=otp,cn=etc,dc=ipa,dc=example
+aci: (targetattr = cn || ipatokenhotpauthwindow || ipatokenhotpsyncwindow || ipatokentotpauthwindow || ipatokentotpsyncwindow)(targetfilter = (objectclass=ipatokenotpconfig))(version 3.0;acl permission:System: Read OTP Configuration;allow (compare,read,search) userdn = ldap:///all;;)
dn: cn=permissions,cn=pbac,dc=ipa,dc=example
aci: (targetattr = member)(targetfilter = (objectclass=ipapermission))(version 3.0;acl permission:System: Modify Privilege Membership;allow (write) groupdn = ldap:///cn=System: Modify Privilege Membership,cn=permissions,cn=pbac,dc=ipa,dc=example;)
dn: dc=ipa,dc=example
diff --git a/API.txt b/API.txt
index e9768bf1e87d6679c439b98ed696b720937099d2..e5e668b0a79a50ea5c2bf9b6d2ae71fb3dbd13f3 100644
--- a/API.txt
+++ b/API.txt
@@ -2599,6 +2599,31 @@ option: Str('version?', exclude='webui')
output: Entry('result', type 'dict', Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
output: Output('summary', (type 'unicode', type 'NoneType'), None)
output: PrimaryKey('value', None, None)
+command: otpconfig_mod
+args: 0,11,3
+option: Str('addattr*', cli_name='addattr', exclude='webui')
+option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
+option: Str('delattr*', cli_name='delattr', exclude='webui')
+option: Int('ipatokenhotpauthwindow', attribute=True, autofill=False, cli_name='hotp_auth_window', minvalue=1, multivalue=False, required=False)
+option: Int('ipatokenhotpsyncwindow', attribute=True, autofill=False, cli_name='hotp_sync_window',
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On 12/02/2014 08:57 PM, Nathaniel McCallum wrote: On Tue, 2014-11-18 at 20:26 +0100, Petr Vobornik wrote: On 13.11.2014 08:53, Martin Kosek wrote: On 11/13/2014 08:51 AM, Nathaniel McCallum wrote: On Thu, 2014-11-13 at 08:48 +0100, Martin Kosek wrote: On 11/12/2014 11:37 PM, Nathaniel McCallum wrote: On Mon, 2014-11-10 at 08:28 +0100, Martin Kosek wrote: On 11/07/2014 04:44 PM, Petr Vobornik wrote: On 7.11.2014 08:58, Martin Kosek wrote: On 11/04/2014 05:17 PM, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 09:34 -0400, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 12:21 +0100, Petr Viktorin wrote: On 10/29/2014 10:37 AM, Martin Kosek wrote: On 10/28/2014 09:59 PM, Nathaniel McCallum wrote: On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote: This patch gives the administrator variables to control the size of the authentication and synchronization windows for OTP tokens. https://fedorahosted.org/freeipa/ticket/4511 NOTE: There is one known issue with this patch which I don't know how to solve. This patch changes the schema in install/share/60ipaconfig.ldif. On an upgrade, all of the new attributeTypes appear correctly. However, the modifications to the pre-existing objectClass do not show up on the server. What am I doing wrong? After modifying ipaGuiConfig manually, everything in this patch works just fine. This new version takes into account the new (proper) OIDs and attribute names. Thanks Nathaniel! The above known issue still remains. Petr3, any idea what could have gone wrong? ObjectClass MAY list extension should work just fine, AFAIK. You added a blank line to the LDIF file. This is an entry separator, so the objectClasses after the blank line don't belong to cn=schema, so they aren't considered in the update. Without the blank line it works fine. Thanks for the catch! Here is a version without the blank line. I forgot to remove the old steps defines. This patch performs this cleanup. I am now wondering, is the global config object really the nest place to add these OTP specific settings? I would prefer not to overload the object and instead: - create new ipaOTPConfig objectclass - add it to cn=otp,$SUFFIX - create otpconfig-mod and otpconfig-show commands to follow an example of dnsconfig-* and trustconfig-* commands IMO, this would allow more flexibility for the OTP settings and would also scale better for the future updates. +1 I will comment the patch as if ^^ would not exist because it will still be needed in the new plugin. Because of ^^ I did not test, just read. 1. Got: install/ui/src/freeipa/serverconfig.js(135): lint warning: extra comma is not recommended in array initializers Please run: jsl -nofilelisting -nosummary -nologo -conf jsl.conf in install/ui directory The goal is no have no warnings and errors. 2. new attrs should be added to 'System: Read Global Configuration' managed permission +1. Though if we go with OTP config, it should be called System: Read OTP Configuration Martin Attached is a new set of patches that replaces this single patch. This now fixes multiple issues. I now create two new entries: * cn=TOTP,cn=Token Config,cn=etc,$SUFFIX * cn=HOTP,cn=Token Config,cn=etc,$SUFFIX There are two corresponding CLI commands: * totpconfig-(show|mod) * hotpconfig-(show|mod) There is no UI support for this yet (pointers welcome). This is designed so that eventually tokens can grow a per-token override, but I have not yet implemented this feature (it should be easy in the future). Additionally, I had to do some shared refactoring to address issues in ipa-otp-lasttoken, which is why all of these are now merged into a single patch set. Nathaniel I'm little confused with a state of reviews. Thierry were some of the patches ACKed in different threads or are they under review (I'm not reviewing DS plugin parts)? Patches 0001, 0002, 0003 are ACKed by Thierry, but not merged. They can and should be merged as they fix an independent bug. Ah, I meant adding the token config to cn=otp,SUFFIX directly, but if we want to make TOTP/HOTP token config as separate entries (to enable future per-token overrides), your approach should make sense. Rather adding Rob to CC for sanity. That would work too. I'm open to that. I am just not sure we should create them as separate plugins, I think the new commands should be rather added to otp plugin directly so that they show in ipa help otptoken instead of adding 2 new topics just for OTP config. I can play with that. Do you plan to change it? I like the idea of a single point of help for OTP but I'm also unsure about the length of the commands. Current solution is also more consistent with a rest of the framework. Would it be something like: otptoken-totpconfig-(show|mod) otptoken-hotpconfig-(show|mod) In the latest patch, I merged totpconfig-* and
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On Wed, 2014-12-03 at 14:43 +0100, Martin Kosek wrote: On 12/02/2014 08:57 PM, Nathaniel McCallum wrote: On Tue, 2014-11-18 at 20:26 +0100, Petr Vobornik wrote: On 13.11.2014 08:53, Martin Kosek wrote: On 11/13/2014 08:51 AM, Nathaniel McCallum wrote: On Thu, 2014-11-13 at 08:48 +0100, Martin Kosek wrote: On 11/12/2014 11:37 PM, Nathaniel McCallum wrote: On Mon, 2014-11-10 at 08:28 +0100, Martin Kosek wrote: On 11/07/2014 04:44 PM, Petr Vobornik wrote: On 7.11.2014 08:58, Martin Kosek wrote: On 11/04/2014 05:17 PM, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 09:34 -0400, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 12:21 +0100, Petr Viktorin wrote: On 10/29/2014 10:37 AM, Martin Kosek wrote: On 10/28/2014 09:59 PM, Nathaniel McCallum wrote: On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote: This patch gives the administrator variables to control the size of the authentication and synchronization windows for OTP tokens. https://fedorahosted.org/freeipa/ticket/4511 NOTE: There is one known issue with this patch which I don't know how to solve. This patch changes the schema in install/share/60ipaconfig.ldif. On an upgrade, all of the new attributeTypes appear correctly. However, the modifications to the pre-existing objectClass do not show up on the server. What am I doing wrong? After modifying ipaGuiConfig manually, everything in this patch works just fine. This new version takes into account the new (proper) OIDs and attribute names. Thanks Nathaniel! The above known issue still remains. Petr3, any idea what could have gone wrong? ObjectClass MAY list extension should work just fine, AFAIK. You added a blank line to the LDIF file. This is an entry separator, so the objectClasses after the blank line don't belong to cn=schema, so they aren't considered in the update. Without the blank line it works fine. Thanks for the catch! Here is a version without the blank line. I forgot to remove the old steps defines. This patch performs this cleanup. I am now wondering, is the global config object really the nest place to add these OTP specific settings? I would prefer not to overload the object and instead: - create new ipaOTPConfig objectclass - add it to cn=otp,$SUFFIX - create otpconfig-mod and otpconfig-show commands to follow an example of dnsconfig-* and trustconfig-* commands IMO, this would allow more flexibility for the OTP settings and would also scale better for the future updates. +1 I will comment the patch as if ^^ would not exist because it will still be needed in the new plugin. Because of ^^ I did not test, just read. 1. Got: install/ui/src/freeipa/serverconfig.js(135): lint warning: extra comma is not recommended in array initializers Please run: jsl -nofilelisting -nosummary -nologo -conf jsl.conf in install/ui directory The goal is no have no warnings and errors. 2. new attrs should be added to 'System: Read Global Configuration' managed permission +1. Though if we go with OTP config, it should be called System: Read OTP Configuration Martin Attached is a new set of patches that replaces this single patch. This now fixes multiple issues. I now create two new entries: * cn=TOTP,cn=Token Config,cn=etc,$SUFFIX * cn=HOTP,cn=Token Config,cn=etc,$SUFFIX There are two corresponding CLI commands: * totpconfig-(show|mod) * hotpconfig-(show|mod) There is no UI support for this yet (pointers welcome). This is designed so that eventually tokens can grow a per-token override, but I have not yet implemented this feature (it should be easy in the future). Additionally, I had to do some shared refactoring to address issues in ipa-otp-lasttoken, which is why all of these are now merged into a single patch set. Nathaniel I'm little confused with a state of reviews. Thierry were some of the patches ACKed in different threads or are they under review (I'm not reviewing DS plugin parts)? Patches 0001, 0002, 0003 are ACKed by Thierry, but not merged. They can and should be merged as they fix an independent bug. Ah, I meant adding the token config to cn=otp,SUFFIX directly, but if we want to make TOTP/HOTP token config as separate entries (to enable future per-token overrides), your approach should make sense. Rather adding Rob to CC for sanity. That would work too. I'm open to that. I am just not sure we should create them as separate plugins, I think the new commands should be rather added to otp plugin directly so that they show in ipa help otptoken instead of adding 2 new topics just for OTP config. I can play with that. Do you plan to change it? I like the idea of a single point of help for OTP but I'm also unsure about the length of the commands.
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On 11/12/2014 11:37 PM, Nathaniel McCallum wrote: On Mon, 2014-11-10 at 08:28 +0100, Martin Kosek wrote: On 11/07/2014 04:44 PM, Petr Vobornik wrote: On 7.11.2014 08:58, Martin Kosek wrote: On 11/04/2014 05:17 PM, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 09:34 -0400, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 12:21 +0100, Petr Viktorin wrote: On 10/29/2014 10:37 AM, Martin Kosek wrote: On 10/28/2014 09:59 PM, Nathaniel McCallum wrote: On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote: This patch gives the administrator variables to control the size of the authentication and synchronization windows for OTP tokens. https://fedorahosted.org/freeipa/ticket/4511 NOTE: There is one known issue with this patch which I don't know how to solve. This patch changes the schema in install/share/60ipaconfig.ldif. On an upgrade, all of the new attributeTypes appear correctly. However, the modifications to the pre-existing objectClass do not show up on the server. What am I doing wrong? After modifying ipaGuiConfig manually, everything in this patch works just fine. This new version takes into account the new (proper) OIDs and attribute names. Thanks Nathaniel! The above known issue still remains. Petr3, any idea what could have gone wrong? ObjectClass MAY list extension should work just fine, AFAIK. You added a blank line to the LDIF file. This is an entry separator, so the objectClasses after the blank line don't belong to cn=schema, so they aren't considered in the update. Without the blank line it works fine. Thanks for the catch! Here is a version without the blank line. I forgot to remove the old steps defines. This patch performs this cleanup. I am now wondering, is the global config object really the nest place to add these OTP specific settings? I would prefer not to overload the object and instead: - create new ipaOTPConfig objectclass - add it to cn=otp,$SUFFIX - create otpconfig-mod and otpconfig-show commands to follow an example of dnsconfig-* and trustconfig-* commands IMO, this would allow more flexibility for the OTP settings and would also scale better for the future updates. +1 I will comment the patch as if ^^ would not exist because it will still be needed in the new plugin. Because of ^^ I did not test, just read. 1. Got: install/ui/src/freeipa/serverconfig.js(135): lint warning: extra comma is not recommended in array initializers Please run: jsl -nofilelisting -nosummary -nologo -conf jsl.conf in install/ui directory The goal is no have no warnings and errors. 2. new attrs should be added to 'System: Read Global Configuration' managed permission +1. Though if we go with OTP config, it should be called System: Read OTP Configuration Martin Attached is a new set of patches that replaces this single patch. This now fixes multiple issues. I now create two new entries: * cn=TOTP,cn=Token Config,cn=etc,$SUFFIX * cn=HOTP,cn=Token Config,cn=etc,$SUFFIX There are two corresponding CLI commands: * totpconfig-(show|mod) * hotpconfig-(show|mod) There is no UI support for this yet (pointers welcome). This is designed so that eventually tokens can grow a per-token override, but I have not yet implemented this feature (it should be easy in the future). Additionally, I had to do some shared refactoring to address issues in ipa-otp-lasttoken, which is why all of these are now merged into a single patch set. Nathaniel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Hello Nathaniel, Very few comments. On patch 0002: Is it possible that we later define a spec with 'dflt' contains OTP_CONFIG_AUTH_TYPE_DISABLED ? If yes it needs to be 32bits. When otp_config_fini is it called ? On patch 0003: In ipa-otp-lasttoken:58 you may use SLAPI_ATTR_OBJECTCLASS (slapi-plugin.h). In ipa-otp-lasttoken:preop_mod , the test is_allowed is done on the original entry (SLAPI_ENTRY_PRE_OP). That is the entry untouched by others BE_PREOP/TXN_BE_PREOP plugins. Is that the entry you want to check ? On patch 0004: In otp_config.c:otp_config_window you may use SLAPI_ATTR_OBJECTCLASS (slapi-plugin.h) in otp_token: bvtod if 'code' contains non digit character ,'out' is not reset before return. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On Mon, 2014-12-01 at 17:46 +0100, thierry bordaz wrote: On 11/12/2014 11:37 PM, Nathaniel McCallum wrote: On Mon, 2014-11-10 at 08:28 +0100, Martin Kosek wrote: On 11/07/2014 04:44 PM, Petr Vobornik wrote: On 7.11.2014 08:58, Martin Kosek wrote: On 11/04/2014 05:17 PM, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 09:34 -0400, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 12:21 +0100, Petr Viktorin wrote: On 10/29/2014 10:37 AM, Martin Kosek wrote: On 10/28/2014 09:59 PM, Nathaniel McCallum wrote: On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote: This patch gives the administrator variables to control the size of the authentication and synchronization windows for OTP tokens. https://fedorahosted.org/freeipa/ticket/4511 NOTE: There is one known issue with this patch which I don't know how to solve. This patch changes the schema in install/share/60ipaconfig.ldif. On an upgrade, all of the new attributeTypes appear correctly. However, the modifications to the pre-existing objectClass do not show up on the server. What am I doing wrong? After modifying ipaGuiConfig manually, everything in this patch works just fine. This new version takes into account the new (proper) OIDs and attribute names. Thanks Nathaniel! The above known issue still remains. Petr3, any idea what could have gone wrong? ObjectClass MAY list extension should work just fine, AFAIK. You added a blank line to the LDIF file. This is an entry separator, so the objectClasses after the blank line don't belong to cn=schema, so they aren't considered in the update. Without the blank line it works fine. Thanks for the catch! Here is a version without the blank line. I forgot to remove the old steps defines. This patch performs this cleanup. I am now wondering, is the global config object really the nest place to add these OTP specific settings? I would prefer not to overload the object and instead: - create new ipaOTPConfig objectclass - add it to cn=otp,$SUFFIX - create otpconfig-mod and otpconfig-show commands to follow an example of dnsconfig-* and trustconfig-* commands IMO, this would allow more flexibility for the OTP settings and would also scale better for the future updates. +1 I will comment the patch as if ^^ would not exist because it will still be needed in the new plugin. Because of ^^ I did not test, just read. 1. Got: install/ui/src/freeipa/serverconfig.js(135): lint warning: extra comma is not recommended in array initializers Please run: jsl -nofilelisting -nosummary -nologo -conf jsl.conf in install/ui directory The goal is no have no warnings and errors. 2. new attrs should be added to 'System: Read Global Configuration' managed permission +1. Though if we go with OTP config, it should be called System: Read OTP Configuration Martin Attached is a new set of patches that replaces this single patch. This now fixes multiple issues. I now create two new entries: * cn=TOTP,cn=Token Config,cn=etc,$SUFFIX * cn=HOTP,cn=Token Config,cn=etc,$SUFFIX There are two corresponding CLI commands: * totpconfig-(show|mod) * hotpconfig-(show|mod) There is no UI support for this yet (pointers welcome). This is designed so that eventually tokens can grow a per-token override, but I have not yet implemented this feature (it should be easy in the future). Additionally, I had to do some shared refactoring to address issues in ipa-otp-lasttoken, which is why all of these are now merged into a single patch set. Nathaniel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Hello Nathaniel, Sorry for this long delay. The patch 0001 is fine for me. Ack I have a question regarding 0002. The function 'otp_config_update' is called in postop in order to 'update' the configuration in case of successful op. In 'update' it can updates 'config_record-value. In case the SLAPI_ENTRY_POST_OP sdn is not the the config_rec-sdn but the SLAPI_TARGET_SDN sdn is the config_rec-sdn , it resets 'config_record'-value to 'config_record-dflt'. Is that the expected effect ? Yes. There are two cases here. 1. If dst is NULL, it means that the config object was deleted. 2. If dst is not NULL
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On Tue, 2014-12-02 at 17:12 +0100, Martin Kosek wrote: On 12/02/2014 04:56 PM, Nathaniel McCallum wrote: On Tue, 2014-12-02 at 14:05 +0100, thierry bordaz wrote: On 11/12/2014 11:37 PM, Nathaniel McCallum wrote: On Mon, 2014-11-10 at 08:28 +0100, Martin Kosek wrote: On 11/07/2014 04:44 PM, Petr Vobornik wrote: On 7.11.2014 08:58, Martin Kosek wrote: On 11/04/2014 05:17 PM, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 09:34 -0400, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 12:21 +0100, Petr Viktorin wrote: On 10/29/2014 10:37 AM, Martin Kosek wrote: On 10/28/2014 09:59 PM, Nathaniel McCallum wrote: On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote: This patch gives the administrator variables to control the size of the authentication and synchronization windows for OTP tokens. https://fedorahosted.org/freeipa/ticket/4511 NOTE: There is one known issue with this patch which I don't know how to solve. This patch changes the schema in install/share/60ipaconfig.ldif. On an upgrade, all of the new attributeTypes appear correctly. However, the modifications to the pre-existing objectClass do not show up on the server. What am I doing wrong? After modifying ipaGuiConfig manually, everything in this patch works just fine. This new version takes into account the new (proper) OIDs and attribute names. Thanks Nathaniel! The above known issue still remains. Petr3, any idea what could have gone wrong? ObjectClass MAY list extension should work just fine, AFAIK. You added a blank line to the LDIF file. This is an entry separator, so the objectClasses after the blank line don't belong to cn=schema, so they aren't considered in the update. Without the blank line it works fine. Thanks for the catch! Here is a version without the blank line. I forgot to remove the old steps defines. This patch performs this cleanup. I am now wondering, is the global config object really the nest place to add these OTP specific settings? I would prefer not to overload the object and instead: - create new ipaOTPConfig objectclass - add it to cn=otp,$SUFFIX - create otpconfig-mod and otpconfig-show commands to follow an example of dnsconfig-* and trustconfig-* commands IMO, this would allow more flexibility for the OTP settings and would also scale better for the future updates. +1 I will comment the patch as if ^^ would not exist because it will still be needed in the new plugin. Because of ^^ I did not test, just read. 1. Got: install/ui/src/freeipa/serverconfig.js(135): lint warning: extra comma is not recommended in array initializers Please run: jsl -nofilelisting -nosummary -nologo -conf jsl.conf in install/ui directory The goal is no have no warnings and errors. 2. new attrs should be added to 'System: Read Global Configuration' managed permission +1. Though if we go with OTP config, it should be called System: Read OTP Configuration Martin Attached is a new set of patches that replaces this single patch. This now fixes multiple issues. I now create two new entries: * cn=TOTP,cn=Token Config,cn=etc,$SUFFIX * cn=HOTP,cn=Token Config,cn=etc,$SUFFIX There are two corresponding CLI commands: * totpconfig-(show|mod) * hotpconfig-(show|mod) There is no UI support for this yet (pointers welcome). This is designed so that eventually tokens can grow a per-token override, but I have not yet implemented this feature (it should be easy in the future). Additionally, I had to do some shared refactoring to address issues in ipa-otp-lasttoken, which is why all of these are now merged into a single patch set. Nathaniel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Hello Nathaniel, Very few comments. Just as a reminder, patch 0001 is already ACKed. On patch 0002: Is it possible that we later define a spec with 'dflt' contains OTP_CONFIG_AUTH_TYPE_DISABLED ? If yes it needs to be 32bits. Fixed. It was just a typo. When otp_config_fini is it called ? Sadly, never. I admit that I am cargo-culting the lack of calling otp_config_fini(). Surely there must be a way to sanely tear this down when 389 shuts down? On patch 0003: In ipa-otp-lasttoken:58 you may use SLAPI_ATTR_OBJECTCLASS (slapi-plugin.h). Fixed. In ipa-otp-lasttoken:preop_mod , the test is_allowed is done on the original entry (SLAPI_ENTRY_PRE_OP). That is the entry untouched by others BE_PREOP/TXN_BE_PREOP plugins. Is that the entry you want to check ? Yes, the code is correct as written. We check to see if a change to the existing
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On 12/02/2014 05:24 PM, Nathaniel McCallum wrote: On Tue, 2014-12-02 at 17:12 +0100, Martin Kosek wrote: On 12/02/2014 04:56 PM, Nathaniel McCallum wrote: On Tue, 2014-12-02 at 14:05 +0100, thierry bordaz wrote: On 11/12/2014 11:37 PM, Nathaniel McCallum wrote: On Mon, 2014-11-10 at 08:28 +0100, Martin Kosek wrote: On 11/07/2014 04:44 PM, Petr Vobornik wrote: On 7.11.2014 08:58, Martin Kosek wrote: On 11/04/2014 05:17 PM, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 09:34 -0400, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 12:21 +0100, Petr Viktorin wrote: On 10/29/2014 10:37 AM, Martin Kosek wrote: On 10/28/2014 09:59 PM, Nathaniel McCallum wrote: On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote: This patch gives the administrator variables to control the size of the authentication and synchronization windows for OTP tokens. https://fedorahosted.org/freeipa/ticket/4511 NOTE: There is one known issue with this patch which I don't know how to solve. This patch changes the schema in install/share/60ipaconfig.ldif. On an upgrade, all of the new attributeTypes appear correctly. However, the modifications to the pre-existing objectClass do not show up on the server. What am I doing wrong? After modifying ipaGuiConfig manually, everything in this patch works just fine. This new version takes into account the new (proper) OIDs and attribute names. Thanks Nathaniel! The above known issue still remains. Petr3, any idea what could have gone wrong? ObjectClass MAY list extension should work just fine, AFAIK. You added a blank line to the LDIF file. This is an entry separator, so the objectClasses after the blank line don't belong to cn=schema, so they aren't considered in the update. Without the blank line it works fine. Thanks for the catch! Here is a version without the blank line. I forgot to remove the old steps defines. This patch performs this cleanup. I am now wondering, is the global config object really the nest place to add these OTP specific settings? I would prefer not to overload the object and instead: - create new ipaOTPConfig objectclass - add it to cn=otp,$SUFFIX - create otpconfig-mod and otpconfig-show commands to follow an example of dnsconfig-* and trustconfig-* commands IMO, this would allow more flexibility for the OTP settings and would also scale better for the future updates. +1 I will comment the patch as if ^^ would not exist because it will still be needed in the new plugin. Because of ^^ I did not test, just read. 1. Got: install/ui/src/freeipa/serverconfig.js(135): lint warning: extra comma is not recommended in array initializers Please run: jsl -nofilelisting -nosummary -nologo -conf jsl.conf in install/ui directory The goal is no have no warnings and errors. 2. new attrs should be added to 'System: Read Global Configuration' managed permission +1. Though if we go with OTP config, it should be called System: Read OTP Configuration Martin Attached is a new set of patches that replaces this single patch. This now fixes multiple issues. I now create two new entries: * cn=TOTP,cn=Token Config,cn=etc,$SUFFIX * cn=HOTP,cn=Token Config,cn=etc,$SUFFIX There are two corresponding CLI commands: * totpconfig-(show|mod) * hotpconfig-(show|mod) There is no UI support for this yet (pointers welcome). This is designed so that eventually tokens can grow a per-token override, but I have not yet implemented this feature (it should be easy in the future). Additionally, I had to do some shared refactoring to address issues in ipa-otp-lasttoken, which is why all of these are now merged into a single patch set. Nathaniel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Hello Nathaniel, Very few comments. Just as a reminder, patch 0001 is already ACKed. On patch 0002: Is it possible that we later define a spec with 'dflt' contains OTP_CONFIG_AUTH_TYPE_DISABLED ? If yes it needs to be 32bits. Fixed. It was just a typo. When otp_config_fini is it called ? Sadly, never. I admit that I am cargo-culting the lack of calling otp_config_fini(). Surely there must be a way to sanely tear this down when 389 shuts down? On patch 0003: In ipa-otp-lasttoken:58 you may use SLAPI_ATTR_OBJECTCLASS (slapi-plugin.h). Fixed. In ipa-otp-lasttoken:preop_mod , the test is_allowed is done on the original entry (SLAPI_ENTRY_PRE_OP). That is the entry untouched by others BE_PREOP/TXN_BE_PREOP plugins. Is that the entry you want to check ? Yes, the code is correct as written. We check to see if a change to the existing state would cause bad behavior. Then, if any such change is attempted (ipa_otp_lasttoken.c:205) we reject it. In the future we might improve this to be more granular regarding the values
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On 12/02/2014 05:39 PM, thierry bordaz wrote: On 12/02/2014 05:24 PM, Nathaniel McCallum wrote: On Tue, 2014-12-02 at 17:12 +0100, Martin Kosek wrote: On 12/02/2014 04:56 PM, Nathaniel McCallum wrote: On Tue, 2014-12-02 at 14:05 +0100, thierry bordaz wrote: On 11/12/2014 11:37 PM, Nathaniel McCallum wrote: On Mon, 2014-11-10 at 08:28 +0100, Martin Kosek wrote: On 11/07/2014 04:44 PM, Petr Vobornik wrote: On 7.11.2014 08:58, Martin Kosek wrote: On 11/04/2014 05:17 PM, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 09:34 -0400, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 12:21 +0100, Petr Viktorin wrote: On 10/29/2014 10:37 AM, Martin Kosek wrote: On 10/28/2014 09:59 PM, Nathaniel McCallum wrote: On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote: This patch gives the administrator variables to control the size of the authentication and synchronization windows for OTP tokens. https://fedorahosted.org/freeipa/ticket/4511 NOTE: There is one known issue with this patch which I don't know how to solve. This patch changes the schema in install/share/60ipaconfig.ldif. On an upgrade, all of the new attributeTypes appear correctly. However, the modifications to the pre-existing objectClass do not show up on the server. What am I doing wrong? After modifying ipaGuiConfig manually, everything in this patch works just fine. This new version takes into account the new (proper) OIDs and attribute names. Thanks Nathaniel! The above known issue still remains. Petr3, any idea what could have gone wrong? ObjectClass MAY list extension should work just fine, AFAIK. You added a blank line to the LDIF file. This is an entry separator, so the objectClasses after the blank line don't belong to cn=schema, so they aren't considered in the update. Without the blank line it works fine. Thanks for the catch! Here is a version without the blank line. I forgot to remove the old steps defines. This patch performs this cleanup. I am now wondering, is the global config object really the nest place to add these OTP specific settings? I would prefer not to overload the object and instead: - create new ipaOTPConfig objectclass - add it to cn=otp,$SUFFIX - create otpconfig-mod and otpconfig-show commands to follow an example of dnsconfig-* and trustconfig-* commands IMO, this would allow more flexibility for the OTP settings and would also scale better for the future updates. +1 I will comment the patch as if ^^ would not exist because it will still be needed in the new plugin. Because of ^^ I did not test, just read. 1. Got: install/ui/src/freeipa/serverconfig.js(135): lint warning: extra comma is not recommended in array initializers Please run: jsl -nofilelisting -nosummary -nologo -conf jsl.conf in install/ui directory The goal is no have no warnings and errors. 2. new attrs should be added to 'System: Read Global Configuration' managed permission +1. Though if we go with OTP config, it should be called System: Read OTP Configuration Martin Attached is a new set of patches that replaces this single patch. This now fixes multiple issues. I now create two new entries: * cn=TOTP,cn=Token Config,cn=etc,$SUFFIX * cn=HOTP,cn=Token Config,cn=etc,$SUFFIX There are two corresponding CLI commands: * totpconfig-(show|mod) * hotpconfig-(show|mod) There is no UI support for this yet (pointers welcome). This is designed so that eventually tokens can grow a per-token override, but I have not yet implemented this feature (it should be easy in the future). Additionally, I had to do some shared refactoring to address issues in ipa-otp-lasttoken, which is why all of these are now merged into a single patch set. Nathaniel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Hello Nathaniel, Very few comments. Just as a reminder, patch 0001 is already ACKed. On patch 0002: Is it possible that we later define a spec with 'dflt' contains OTP_CONFIG_AUTH_TYPE_DISABLED ? If yes it needs to be 32bits. Fixed. It was just a typo. When otp_config_fini is it called ? Sadly, never. I admit that I am cargo-culting the lack of calling otp_config_fini(). Surely there must be a way to sanely tear this down when 389 shuts down? On patch 0003: In ipa-otp-lasttoken:58 you may use SLAPI_ATTR_OBJECTCLASS (slapi-plugin.h). Fixed. In ipa-otp-lasttoken:preop_mod , the test is_allowed is done on the original entry (SLAPI_ENTRY_PRE_OP). That is the entry untouched by others BE_PREOP/TXN_BE_PREOP plugins. Is that the entry you want to check ? Yes, the code is correct as written. We check to see if a change to the existing state would cause bad behavior. Then, if any such change is attempted (ipa_otp_lasttoken.c:205) we reject it. In the future we might improve this to be more
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On Tue, 2014-12-02 at 17:56 +0100, Petr Vobornik wrote: On 12/02/2014 05:39 PM, thierry bordaz wrote: On 12/02/2014 05:24 PM, Nathaniel McCallum wrote: On Tue, 2014-12-02 at 17:12 +0100, Martin Kosek wrote: On 12/02/2014 04:56 PM, Nathaniel McCallum wrote: On Tue, 2014-12-02 at 14:05 +0100, thierry bordaz wrote: On 11/12/2014 11:37 PM, Nathaniel McCallum wrote: On Mon, 2014-11-10 at 08:28 +0100, Martin Kosek wrote: On 11/07/2014 04:44 PM, Petr Vobornik wrote: On 7.11.2014 08:58, Martin Kosek wrote: On 11/04/2014 05:17 PM, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 09:34 -0400, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 12:21 +0100, Petr Viktorin wrote: On 10/29/2014 10:37 AM, Martin Kosek wrote: On 10/28/2014 09:59 PM, Nathaniel McCallum wrote: On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote: This patch gives the administrator variables to control the size of the authentication and synchronization windows for OTP tokens. https://fedorahosted.org/freeipa/ticket/4511 NOTE: There is one known issue with this patch which I don't know how to solve. This patch changes the schema in install/share/60ipaconfig.ldif. On an upgrade, all of the new attributeTypes appear correctly. However, the modifications to the pre-existing objectClass do not show up on the server. What am I doing wrong? After modifying ipaGuiConfig manually, everything in this patch works just fine. This new version takes into account the new (proper) OIDs and attribute names. Thanks Nathaniel! The above known issue still remains. Petr3, any idea what could have gone wrong? ObjectClass MAY list extension should work just fine, AFAIK. You added a blank line to the LDIF file. This is an entry separator, so the objectClasses after the blank line don't belong to cn=schema, so they aren't considered in the update. Without the blank line it works fine. Thanks for the catch! Here is a version without the blank line. I forgot to remove the old steps defines. This patch performs this cleanup. I am now wondering, is the global config object really the nest place to add these OTP specific settings? I would prefer not to overload the object and instead: - create new ipaOTPConfig objectclass - add it to cn=otp,$SUFFIX - create otpconfig-mod and otpconfig-show commands to follow an example of dnsconfig-* and trustconfig-* commands IMO, this would allow more flexibility for the OTP settings and would also scale better for the future updates. +1 I will comment the patch as if ^^ would not exist because it will still be needed in the new plugin. Because of ^^ I did not test, just read. 1. Got: install/ui/src/freeipa/serverconfig.js(135): lint warning: extra comma is not recommended in array initializers Please run: jsl -nofilelisting -nosummary -nologo -conf jsl.conf in install/ui directory The goal is no have no warnings and errors. 2. new attrs should be added to 'System: Read Global Configuration' managed permission +1. Though if we go with OTP config, it should be called System: Read OTP Configuration Martin Attached is a new set of patches that replaces this single patch. This now fixes multiple issues. I now create two new entries: * cn=TOTP,cn=Token Config,cn=etc,$SUFFIX * cn=HOTP,cn=Token Config,cn=etc,$SUFFIX There are two corresponding CLI commands: * totpconfig-(show|mod) * hotpconfig-(show|mod) There is no UI support for this yet (pointers welcome). This is designed so that eventually tokens can grow a per-token override, but I have not yet implemented this feature (it should be easy in the future). Additionally, I had to do some shared refactoring to address issues in ipa-otp-lasttoken, which is why all of these are now merged into a single patch set. Nathaniel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Hello Nathaniel, Very few comments. Just as a reminder, patch 0001 is already ACKed. On patch 0002: Is it possible that we later define a spec with 'dflt' contains OTP_CONFIG_AUTH_TYPE_DISABLED ? If yes it needs to be 32bits. Fixed. It was just a typo. When otp_config_fini is it called ? Sadly, never. I admit that I am cargo-culting the lack of calling otp_config_fini(). Surely there must be a way to sanely tear this down when 389 shuts down? On patch 0003: In ipa-otp-lasttoken:58 you may use SLAPI_ATTR_OBJECTCLASS (slapi-plugin.h). Fixed. In ipa-otp-lasttoken:preop_mod , the test is_allowed is done on the original entry (SLAPI_ENTRY_PRE_OP). That is the entry untouched by others
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On Tue, 2014-12-02 at 12:20 -0500, Nathaniel McCallum wrote: On Tue, 2014-12-02 at 17:56 +0100, Petr Vobornik wrote: On 12/02/2014 05:39 PM, thierry bordaz wrote: On 12/02/2014 05:24 PM, Nathaniel McCallum wrote: On Tue, 2014-12-02 at 17:12 +0100, Martin Kosek wrote: On 12/02/2014 04:56 PM, Nathaniel McCallum wrote: On Tue, 2014-12-02 at 14:05 +0100, thierry bordaz wrote: On 11/12/2014 11:37 PM, Nathaniel McCallum wrote: On Mon, 2014-11-10 at 08:28 +0100, Martin Kosek wrote: On 11/07/2014 04:44 PM, Petr Vobornik wrote: On 7.11.2014 08:58, Martin Kosek wrote: On 11/04/2014 05:17 PM, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 09:34 -0400, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 12:21 +0100, Petr Viktorin wrote: On 10/29/2014 10:37 AM, Martin Kosek wrote: On 10/28/2014 09:59 PM, Nathaniel McCallum wrote: On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote: This patch gives the administrator variables to control the size of the authentication and synchronization windows for OTP tokens. https://fedorahosted.org/freeipa/ticket/4511 NOTE: There is one known issue with this patch which I don't know how to solve. This patch changes the schema in install/share/60ipaconfig.ldif. On an upgrade, all of the new attributeTypes appear correctly. However, the modifications to the pre-existing objectClass do not show up on the server. What am I doing wrong? After modifying ipaGuiConfig manually, everything in this patch works just fine. This new version takes into account the new (proper) OIDs and attribute names. Thanks Nathaniel! The above known issue still remains. Petr3, any idea what could have gone wrong? ObjectClass MAY list extension should work just fine, AFAIK. You added a blank line to the LDIF file. This is an entry separator, so the objectClasses after the blank line don't belong to cn=schema, so they aren't considered in the update. Without the blank line it works fine. Thanks for the catch! Here is a version without the blank line. I forgot to remove the old steps defines. This patch performs this cleanup. I am now wondering, is the global config object really the nest place to add these OTP specific settings? I would prefer not to overload the object and instead: - create new ipaOTPConfig objectclass - add it to cn=otp,$SUFFIX - create otpconfig-mod and otpconfig-show commands to follow an example of dnsconfig-* and trustconfig-* commands IMO, this would allow more flexibility for the OTP settings and would also scale better for the future updates. +1 I will comment the patch as if ^^ would not exist because it will still be needed in the new plugin. Because of ^^ I did not test, just read. 1. Got: install/ui/src/freeipa/serverconfig.js(135): lint warning: extra comma is not recommended in array initializers Please run: jsl -nofilelisting -nosummary -nologo -conf jsl.conf in install/ui directory The goal is no have no warnings and errors. 2. new attrs should be added to 'System: Read Global Configuration' managed permission +1. Though if we go with OTP config, it should be called System: Read OTP Configuration Martin Attached is a new set of patches that replaces this single patch. This now fixes multiple issues. I now create two new entries: * cn=TOTP,cn=Token Config,cn=etc,$SUFFIX * cn=HOTP,cn=Token Config,cn=etc,$SUFFIX There are two corresponding CLI commands: * totpconfig-(show|mod) * hotpconfig-(show|mod) There is no UI support for this yet (pointers welcome). This is designed so that eventually tokens can grow a per-token override, but I have not yet implemented this feature (it should be easy in the future). Additionally, I had to do some shared refactoring to address issues in ipa-otp-lasttoken, which is why all of these are now merged into a single patch set. Nathaniel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Hello Nathaniel, Very few comments. Just as a reminder, patch 0001 is already ACKed. On patch 0002: Is it possible that we later define a spec with 'dflt' contains OTP_CONFIG_AUTH_TYPE_DISABLED ? If yes it needs to be 32bits. Fixed. It was just a typo. When otp_config_fini is it called ? Sadly, never. I admit that I am cargo-culting the lack of calling otp_config_fini(). Surely there must be a way to sanely tear this down when 389 shuts down? On patch 0003: In ipa-otp-lasttoken:58 you may use SLAPI_ATTR_OBJECTCLASS
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On 11/12/2014 11:37 PM, Nathaniel McCallum wrote: On Mon, 2014-11-10 at 08:28 +0100, Martin Kosek wrote: On 11/07/2014 04:44 PM, Petr Vobornik wrote: On 7.11.2014 08:58, Martin Kosek wrote: On 11/04/2014 05:17 PM, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 09:34 -0400, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 12:21 +0100, Petr Viktorin wrote: On 10/29/2014 10:37 AM, Martin Kosek wrote: On 10/28/2014 09:59 PM, Nathaniel McCallum wrote: On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote: This patch gives the administrator variables to control the size of the authentication and synchronization windows for OTP tokens. https://fedorahosted.org/freeipa/ticket/4511 NOTE: There is one known issue with this patch which I don't know how to solve. This patch changes the schema in install/share/60ipaconfig.ldif. On an upgrade, all of the new attributeTypes appear correctly. However, the modifications to the pre-existing objectClass do not show up on the server. What am I doing wrong? After modifying ipaGuiConfig manually, everything in this patch works just fine. This new version takes into account the new (proper) OIDs and attribute names. Thanks Nathaniel! The above known issue still remains. Petr3, any idea what could have gone wrong? ObjectClass MAY list extension should work just fine, AFAIK. You added a blank line to the LDIF file. This is an entry separator, so the objectClasses after the blank line don't belong to cn=schema, so they aren't considered in the update. Without the blank line it works fine. Thanks for the catch! Here is a version without the blank line. I forgot to remove the old steps defines. This patch performs this cleanup. I am now wondering, is the global config object really the nest place to add these OTP specific settings? I would prefer not to overload the object and instead: - create new ipaOTPConfig objectclass - add it to cn=otp,$SUFFIX - create otpconfig-mod and otpconfig-show commands to follow an example of dnsconfig-* and trustconfig-* commands IMO, this would allow more flexibility for the OTP settings and would also scale better for the future updates. +1 I will comment the patch as if ^^ would not exist because it will still be needed in the new plugin. Because of ^^ I did not test, just read. 1. Got: install/ui/src/freeipa/serverconfig.js(135): lint warning: extra comma is not recommended in array initializers Please run: jsl -nofilelisting -nosummary -nologo -conf jsl.conf in install/ui directory The goal is no have no warnings and errors. 2. new attrs should be added to 'System: Read Global Configuration' managed permission +1. Though if we go with OTP config, it should be called System: Read OTP Configuration Martin Attached is a new set of patches that replaces this single patch. This now fixes multiple issues. I now create two new entries: * cn=TOTP,cn=Token Config,cn=etc,$SUFFIX * cn=HOTP,cn=Token Config,cn=etc,$SUFFIX There are two corresponding CLI commands: * totpconfig-(show|mod) * hotpconfig-(show|mod) There is no UI support for this yet (pointers welcome). This is designed so that eventually tokens can grow a per-token override, but I have not yet implemented this feature (it should be easy in the future). Additionally, I had to do some shared refactoring to address issues in ipa-otp-lasttoken, which is why all of these are now merged into a single patch set. Nathaniel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Hello Nathaniel, Sorry for this long delay. The patch 0001 is fine for me. Ack I have a question regarding 0002. The function 'otp_config_update' is called in postop in order to 'update' the configuration in case of successful op. In 'update' it can updates 'config_record-value. In case the SLAPI_ENTRY_POST_OP sdn is not the the config_rec-sdn but the SLAPI_TARGET_SDN sdn is the config_rec-sdn , it resets 'config_record'-value to 'config_record-dflt'. Is that the expected effect ? thanks thierry ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On 11/18/2014 08:26 PM, Petr Vobornik wrote:
On 13.11.2014 08:53, Martin Kosek wrote:
On 11/13/2014 08:51 AM, Nathaniel McCallum wrote:
On Thu, 2014-11-13 at 08:48 +0100, Martin Kosek wrote:
On 11/12/2014 11:37 PM, Nathaniel McCallum wrote:
On Mon, 2014-11-10 at 08:28 +0100, Martin Kosek wrote:
On 11/07/2014 04:44 PM, Petr Vobornik wrote:
On 7.11.2014 08:58, Martin Kosek wrote:
On 11/04/2014 05:17 PM, Nathaniel McCallum wrote:
On Wed, 2014-10-29 at 09:34 -0400, Nathaniel McCallum wrote:
On Wed, 2014-10-29 at 12:21 +0100, Petr Viktorin wrote:
On 10/29/2014 10:37 AM, Martin Kosek wrote:
On 10/28/2014 09:59 PM, Nathaniel McCallum wrote:
On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote:
This patch gives the administrator variables to control
the size of
the authentication and synchronization windows for OTP
tokens.
https://fedorahosted.org/freeipa/ticket/4511
NOTE: There is one known issue with this patch which I
don't know
how to
solve. This patch changes the schema in
install/share/60ipaconfig.ldif.
On an upgrade, all of the new attributeTypes appear
correctly.
However,
the modifications to the pre-existing objectClass do not
show up
on the
server. What am I doing wrong?
After modifying ipaGuiConfig manually, everything in this
patch
works
just fine.
This new version takes into account the new (proper) OIDs and
attribute
names.
Thanks Nathaniel!
The above known issue still remains.
Petr3, any idea what could have gone wrong? ObjectClass MAY
list
extension
should work just fine, AFAIK.
You added a blank line to the LDIF file. This is an entry
separator, so
the objectClasses after the blank line don't belong to
cn=schema, so
they aren't considered in the update.
Without the blank line it works fine.
Thanks for the catch!
Here is a version without the blank line.
I forgot to remove the old steps defines. This patch performs
this
cleanup.
I am now wondering, is the global config object really the nest
place to
add these OTP specific settings?
I would prefer not to overload the object and instead:
- create new ipaOTPConfig objectclass
- add it to cn=otp,$SUFFIX
- create otpconfig-mod and otpconfig-show commands to follow an
example
of dnsconfig-* and trustconfig-* commands
IMO, this would allow more flexibility for the OTP settings and
would
also scale better for the future updates.
+1
I will comment the patch as if ^^ would not exist because it
will still be
needed in the new plugin.
Because of ^^ I did not test, just read.
1. Got:
install/ui/src/freeipa/serverconfig.js(135): lint warning: extra
comma is not
recommended in array initializers
Please run:
jsl -nofilelisting -nosummary -nologo -conf jsl.conf
in install/ui directory
The goal is no have no warnings and errors.
2. new attrs should be added to 'System: Read Global
Configuration' managed
permission
+1. Though if we go with OTP config, it should be called
System: Read OTP Configuration
Martin
Attached is a new set of patches that replaces this single patch.
This
now fixes multiple issues.
I now create two new entries:
* cn=TOTP,cn=Token Config,cn=etc,$SUFFIX
* cn=HOTP,cn=Token Config,cn=etc,$SUFFIX
There are two corresponding CLI commands:
* totpconfig-(show|mod)
* hotpconfig-(show|mod)
There is no UI support for this yet (pointers welcome).
This is designed so that eventually tokens can grow a per-token
override, but I have not yet implemented this feature (it should
be easy
in the future).
Additionally, I had to do some shared refactoring to address
issues in
ipa-otp-lasttoken, which is why all of these are now merged into a
single patch set.
Nathaniel
I'm little confused with a state of reviews. Thierry were some of the
patches ACKed in different threads or are they under review (I'm not
reviewing DS plugin parts)?
I am sorry for the long delay...
I am having difficulties to apply the patches. I am on master branch.
For example I see those errors:
git apply -v /tmp/0001-Preliminary-refactoring-of-libotp-files.patch
Checking patch
daemons/ipa-slapi-plugins/ipa-otp-lasttoken/Makefile.am...
Checking patch
daemons/ipa-slapi-plugins/ipa-otp-lasttoken/ipa_otp_lasttoken.c...
error: while searching for:
# include config.h
#endif
#include libotp.h
#include time.h
#include util.h
#define PLUGIN_NAME ipa-otp-lasttoken
#define LOG(sev, ...) \
slapi_log_error(SLAPI_LOG_ ## sev, PLUGIN_NAME, \
%s: %s\n, __func__, __VA_ARGS__), -1
static void *plugin_id;
static const Slapi_PluginDesc preop_desc = {
error: patch failed:
daemons/ipa-slapi-plugins/ipa-otp-lasttoken/ipa_otp_lasttoken.c:41
error:
daemons/ipa-slapi-plugins/ipa-otp-lasttoken/ipa_otp_lasttoken.c:
patch does not apply
Checking patch daemons/ipa-slapi-plugins/ipa-pwd-extop/Makefile.am...
error: while searching for:
AM_CPPFLAGS =\
-I.
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On 13.11.2014 08:53, Martin Kosek wrote: On 11/13/2014 08:51 AM, Nathaniel McCallum wrote: On Thu, 2014-11-13 at 08:48 +0100, Martin Kosek wrote: On 11/12/2014 11:37 PM, Nathaniel McCallum wrote: On Mon, 2014-11-10 at 08:28 +0100, Martin Kosek wrote: On 11/07/2014 04:44 PM, Petr Vobornik wrote: On 7.11.2014 08:58, Martin Kosek wrote: On 11/04/2014 05:17 PM, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 09:34 -0400, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 12:21 +0100, Petr Viktorin wrote: On 10/29/2014 10:37 AM, Martin Kosek wrote: On 10/28/2014 09:59 PM, Nathaniel McCallum wrote: On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote: This patch gives the administrator variables to control the size of the authentication and synchronization windows for OTP tokens. https://fedorahosted.org/freeipa/ticket/4511 NOTE: There is one known issue with this patch which I don't know how to solve. This patch changes the schema in install/share/60ipaconfig.ldif. On an upgrade, all of the new attributeTypes appear correctly. However, the modifications to the pre-existing objectClass do not show up on the server. What am I doing wrong? After modifying ipaGuiConfig manually, everything in this patch works just fine. This new version takes into account the new (proper) OIDs and attribute names. Thanks Nathaniel! The above known issue still remains. Petr3, any idea what could have gone wrong? ObjectClass MAY list extension should work just fine, AFAIK. You added a blank line to the LDIF file. This is an entry separator, so the objectClasses after the blank line don't belong to cn=schema, so they aren't considered in the update. Without the blank line it works fine. Thanks for the catch! Here is a version without the blank line. I forgot to remove the old steps defines. This patch performs this cleanup. I am now wondering, is the global config object really the nest place to add these OTP specific settings? I would prefer not to overload the object and instead: - create new ipaOTPConfig objectclass - add it to cn=otp,$SUFFIX - create otpconfig-mod and otpconfig-show commands to follow an example of dnsconfig-* and trustconfig-* commands IMO, this would allow more flexibility for the OTP settings and would also scale better for the future updates. +1 I will comment the patch as if ^^ would not exist because it will still be needed in the new plugin. Because of ^^ I did not test, just read. 1. Got: install/ui/src/freeipa/serverconfig.js(135): lint warning: extra comma is not recommended in array initializers Please run: jsl -nofilelisting -nosummary -nologo -conf jsl.conf in install/ui directory The goal is no have no warnings and errors. 2. new attrs should be added to 'System: Read Global Configuration' managed permission +1. Though if we go with OTP config, it should be called System: Read OTP Configuration Martin Attached is a new set of patches that replaces this single patch. This now fixes multiple issues. I now create two new entries: * cn=TOTP,cn=Token Config,cn=etc,$SUFFIX * cn=HOTP,cn=Token Config,cn=etc,$SUFFIX There are two corresponding CLI commands: * totpconfig-(show|mod) * hotpconfig-(show|mod) There is no UI support for this yet (pointers welcome). This is designed so that eventually tokens can grow a per-token override, but I have not yet implemented this feature (it should be easy in the future). Additionally, I had to do some shared refactoring to address issues in ipa-otp-lasttoken, which is why all of these are now merged into a single patch set. Nathaniel I'm little confused with a state of reviews. Thierry were some of the patches ACKed in different threads or are they under review (I'm not reviewing DS plugin parts)? Ah, I meant adding the token config to cn=otp,SUFFIX directly, but if we want to make TOTP/HOTP token config as separate entries (to enable future per-token overrides), your approach should make sense. Rather adding Rob to CC for sanity. That would work too. I'm open to that. I am just not sure we should create them as separate plugins, I think the new commands should be rather added to otp plugin directly so that they show in ipa help otptoken instead of adding 2 new topics just for OTP config. I can play with that. Do you plan to change it? I like the idea of a single point of help for OTP but I'm also unsure about the length of the commands. Current solution is also more consistent with a rest of the framework. Would it be something like: otptoken-totpconfig-(show|mod) otptoken-hotpconfig-(show|mod) Maybe it would be better to introduce more help topics for otp. This concept is used for HBAC already: $ ipa help hbac hbacsvcgroup HBAC Service Groups hbacsvc HBAC Services hbacrule Host-based access control $ ipa help hbacrule Host-based access control ... a lot of text So we could introduce otp umbrella topic: $ ipa help otp opttoken
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On 11/12/2014 11:37 PM, Nathaniel McCallum wrote: On Mon, 2014-11-10 at 08:28 +0100, Martin Kosek wrote: On 11/07/2014 04:44 PM, Petr Vobornik wrote: On 7.11.2014 08:58, Martin Kosek wrote: On 11/04/2014 05:17 PM, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 09:34 -0400, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 12:21 +0100, Petr Viktorin wrote: On 10/29/2014 10:37 AM, Martin Kosek wrote: On 10/28/2014 09:59 PM, Nathaniel McCallum wrote: On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote: This patch gives the administrator variables to control the size of the authentication and synchronization windows for OTP tokens. https://fedorahosted.org/freeipa/ticket/4511 NOTE: There is one known issue with this patch which I don't know how to solve. This patch changes the schema in install/share/60ipaconfig.ldif. On an upgrade, all of the new attributeTypes appear correctly. However, the modifications to the pre-existing objectClass do not show up on the server. What am I doing wrong? After modifying ipaGuiConfig manually, everything in this patch works just fine. This new version takes into account the new (proper) OIDs and attribute names. Thanks Nathaniel! The above known issue still remains. Petr3, any idea what could have gone wrong? ObjectClass MAY list extension should work just fine, AFAIK. You added a blank line to the LDIF file. This is an entry separator, so the objectClasses after the blank line don't belong to cn=schema, so they aren't considered in the update. Without the blank line it works fine. Thanks for the catch! Here is a version without the blank line. I forgot to remove the old steps defines. This patch performs this cleanup. I am now wondering, is the global config object really the nest place to add these OTP specific settings? I would prefer not to overload the object and instead: - create new ipaOTPConfig objectclass - add it to cn=otp,$SUFFIX - create otpconfig-mod and otpconfig-show commands to follow an example of dnsconfig-* and trustconfig-* commands IMO, this would allow more flexibility for the OTP settings and would also scale better for the future updates. +1 I will comment the patch as if ^^ would not exist because it will still be needed in the new plugin. Because of ^^ I did not test, just read. 1. Got: install/ui/src/freeipa/serverconfig.js(135): lint warning: extra comma is not recommended in array initializers Please run: jsl -nofilelisting -nosummary -nologo -conf jsl.conf in install/ui directory The goal is no have no warnings and errors. 2. new attrs should be added to 'System: Read Global Configuration' managed permission +1. Though if we go with OTP config, it should be called System: Read OTP Configuration Martin Attached is a new set of patches that replaces this single patch. This now fixes multiple issues. I now create two new entries: * cn=TOTP,cn=Token Config,cn=etc,$SUFFIX * cn=HOTP,cn=Token Config,cn=etc,$SUFFIX There are two corresponding CLI commands: * totpconfig-(show|mod) * hotpconfig-(show|mod) There is no UI support for this yet (pointers welcome). This is designed so that eventually tokens can grow a per-token override, but I have not yet implemented this feature (it should be easy in the future). Additionally, I had to do some shared refactoring to address issues in ipa-otp-lasttoken, which is why all of these are now merged into a single patch set. Nathaniel Ah, I meant adding the token config to cn=otp,SUFFIX directly, but if we want to make TOTP/HOTP token config as separate entries (to enable future per-token overrides), your approach should make sense. Rather adding Rob to CC for sanity. I am just not sure we should create them as separate plugins, I think the new commands should be rather added to otp plugin directly so that they show in ipa help otptoken instead of adding 2 new topics just for OTP config. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On Thu, 2014-11-13 at 08:48 +0100, Martin Kosek wrote: On 11/12/2014 11:37 PM, Nathaniel McCallum wrote: On Mon, 2014-11-10 at 08:28 +0100, Martin Kosek wrote: On 11/07/2014 04:44 PM, Petr Vobornik wrote: On 7.11.2014 08:58, Martin Kosek wrote: On 11/04/2014 05:17 PM, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 09:34 -0400, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 12:21 +0100, Petr Viktorin wrote: On 10/29/2014 10:37 AM, Martin Kosek wrote: On 10/28/2014 09:59 PM, Nathaniel McCallum wrote: On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote: This patch gives the administrator variables to control the size of the authentication and synchronization windows for OTP tokens. https://fedorahosted.org/freeipa/ticket/4511 NOTE: There is one known issue with this patch which I don't know how to solve. This patch changes the schema in install/share/60ipaconfig.ldif. On an upgrade, all of the new attributeTypes appear correctly. However, the modifications to the pre-existing objectClass do not show up on the server. What am I doing wrong? After modifying ipaGuiConfig manually, everything in this patch works just fine. This new version takes into account the new (proper) OIDs and attribute names. Thanks Nathaniel! The above known issue still remains. Petr3, any idea what could have gone wrong? ObjectClass MAY list extension should work just fine, AFAIK. You added a blank line to the LDIF file. This is an entry separator, so the objectClasses after the blank line don't belong to cn=schema, so they aren't considered in the update. Without the blank line it works fine. Thanks for the catch! Here is a version without the blank line. I forgot to remove the old steps defines. This patch performs this cleanup. I am now wondering, is the global config object really the nest place to add these OTP specific settings? I would prefer not to overload the object and instead: - create new ipaOTPConfig objectclass - add it to cn=otp,$SUFFIX - create otpconfig-mod and otpconfig-show commands to follow an example of dnsconfig-* and trustconfig-* commands IMO, this would allow more flexibility for the OTP settings and would also scale better for the future updates. +1 I will comment the patch as if ^^ would not exist because it will still be needed in the new plugin. Because of ^^ I did not test, just read. 1. Got: install/ui/src/freeipa/serverconfig.js(135): lint warning: extra comma is not recommended in array initializers Please run: jsl -nofilelisting -nosummary -nologo -conf jsl.conf in install/ui directory The goal is no have no warnings and errors. 2. new attrs should be added to 'System: Read Global Configuration' managed permission +1. Though if we go with OTP config, it should be called System: Read OTP Configuration Martin Attached is a new set of patches that replaces this single patch. This now fixes multiple issues. I now create two new entries: * cn=TOTP,cn=Token Config,cn=etc,$SUFFIX * cn=HOTP,cn=Token Config,cn=etc,$SUFFIX There are two corresponding CLI commands: * totpconfig-(show|mod) * hotpconfig-(show|mod) There is no UI support for this yet (pointers welcome). This is designed so that eventually tokens can grow a per-token override, but I have not yet implemented this feature (it should be easy in the future). Additionally, I had to do some shared refactoring to address issues in ipa-otp-lasttoken, which is why all of these are now merged into a single patch set. Nathaniel Ah, I meant adding the token config to cn=otp,SUFFIX directly, but if we want to make TOTP/HOTP token config as separate entries (to enable future per-token overrides), your approach should make sense. Rather adding Rob to CC for sanity. That would work too. I'm open to that. I am just not sure we should create them as separate plugins, I think the new commands should be rather added to otp plugin directly so that they show in ipa help otptoken instead of adding 2 new topics just for OTP config. I can play with that. Nathaniel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On 11/13/2014 08:51 AM, Nathaniel McCallum wrote: On Thu, 2014-11-13 at 08:48 +0100, Martin Kosek wrote: On 11/12/2014 11:37 PM, Nathaniel McCallum wrote: On Mon, 2014-11-10 at 08:28 +0100, Martin Kosek wrote: On 11/07/2014 04:44 PM, Petr Vobornik wrote: On 7.11.2014 08:58, Martin Kosek wrote: On 11/04/2014 05:17 PM, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 09:34 -0400, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 12:21 +0100, Petr Viktorin wrote: On 10/29/2014 10:37 AM, Martin Kosek wrote: On 10/28/2014 09:59 PM, Nathaniel McCallum wrote: On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote: This patch gives the administrator variables to control the size of the authentication and synchronization windows for OTP tokens. https://fedorahosted.org/freeipa/ticket/4511 NOTE: There is one known issue with this patch which I don't know how to solve. This patch changes the schema in install/share/60ipaconfig.ldif. On an upgrade, all of the new attributeTypes appear correctly. However, the modifications to the pre-existing objectClass do not show up on the server. What am I doing wrong? After modifying ipaGuiConfig manually, everything in this patch works just fine. This new version takes into account the new (proper) OIDs and attribute names. Thanks Nathaniel! The above known issue still remains. Petr3, any idea what could have gone wrong? ObjectClass MAY list extension should work just fine, AFAIK. You added a blank line to the LDIF file. This is an entry separator, so the objectClasses after the blank line don't belong to cn=schema, so they aren't considered in the update. Without the blank line it works fine. Thanks for the catch! Here is a version without the blank line. I forgot to remove the old steps defines. This patch performs this cleanup. I am now wondering, is the global config object really the nest place to add these OTP specific settings? I would prefer not to overload the object and instead: - create new ipaOTPConfig objectclass - add it to cn=otp,$SUFFIX - create otpconfig-mod and otpconfig-show commands to follow an example of dnsconfig-* and trustconfig-* commands IMO, this would allow more flexibility for the OTP settings and would also scale better for the future updates. +1 I will comment the patch as if ^^ would not exist because it will still be needed in the new plugin. Because of ^^ I did not test, just read. 1. Got: install/ui/src/freeipa/serverconfig.js(135): lint warning: extra comma is not recommended in array initializers Please run: jsl -nofilelisting -nosummary -nologo -conf jsl.conf in install/ui directory The goal is no have no warnings and errors. 2. new attrs should be added to 'System: Read Global Configuration' managed permission +1. Though if we go with OTP config, it should be called System: Read OTP Configuration Martin Attached is a new set of patches that replaces this single patch. This now fixes multiple issues. I now create two new entries: * cn=TOTP,cn=Token Config,cn=etc,$SUFFIX * cn=HOTP,cn=Token Config,cn=etc,$SUFFIX There are two corresponding CLI commands: * totpconfig-(show|mod) * hotpconfig-(show|mod) There is no UI support for this yet (pointers welcome). This is designed so that eventually tokens can grow a per-token override, but I have not yet implemented this feature (it should be easy in the future). Additionally, I had to do some shared refactoring to address issues in ipa-otp-lasttoken, which is why all of these are now merged into a single patch set. Nathaniel Ah, I meant adding the token config to cn=otp,SUFFIX directly, but if we want to make TOTP/HOTP token config as separate entries (to enable future per-token overrides), your approach should make sense. Rather adding Rob to CC for sanity. That would work too. I'm open to that. I am just not sure we should create them as separate plugins, I think the new commands should be rather added to otp plugin directly so that they show in ipa help otptoken instead of adding 2 new topics just for OTP config. I can play with that. Nathaniel No worries ATM, you can wait for proper review. I was just looking at the new API to make sure we are on the same page - we seem to mostly are. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On 11/07/2014 04:44 PM, Petr Vobornik wrote: On 7.11.2014 08:58, Martin Kosek wrote: On 11/04/2014 05:17 PM, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 09:34 -0400, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 12:21 +0100, Petr Viktorin wrote: On 10/29/2014 10:37 AM, Martin Kosek wrote: On 10/28/2014 09:59 PM, Nathaniel McCallum wrote: On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote: This patch gives the administrator variables to control the size of the authentication and synchronization windows for OTP tokens. https://fedorahosted.org/freeipa/ticket/4511 NOTE: There is one known issue with this patch which I don't know how to solve. This patch changes the schema in install/share/60ipaconfig.ldif. On an upgrade, all of the new attributeTypes appear correctly. However, the modifications to the pre-existing objectClass do not show up on the server. What am I doing wrong? After modifying ipaGuiConfig manually, everything in this patch works just fine. This new version takes into account the new (proper) OIDs and attribute names. Thanks Nathaniel! The above known issue still remains. Petr3, any idea what could have gone wrong? ObjectClass MAY list extension should work just fine, AFAIK. You added a blank line to the LDIF file. This is an entry separator, so the objectClasses after the blank line don't belong to cn=schema, so they aren't considered in the update. Without the blank line it works fine. Thanks for the catch! Here is a version without the blank line. I forgot to remove the old steps defines. This patch performs this cleanup. I am now wondering, is the global config object really the nest place to add these OTP specific settings? I would prefer not to overload the object and instead: - create new ipaOTPConfig objectclass - add it to cn=otp,$SUFFIX - create otpconfig-mod and otpconfig-show commands to follow an example of dnsconfig-* and trustconfig-* commands IMO, this would allow more flexibility for the OTP settings and would also scale better for the future updates. +1 I will comment the patch as if ^^ would not exist because it will still be needed in the new plugin. Because of ^^ I did not test, just read. 1. Got: install/ui/src/freeipa/serverconfig.js(135): lint warning: extra comma is not recommended in array initializers Please run: jsl -nofilelisting -nosummary -nologo -conf jsl.conf in install/ui directory The goal is no have no warnings and errors. 2. new attrs should be added to 'System: Read Global Configuration' managed permission +1. Though if we go with OTP config, it should be called System: Read OTP Configuration Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On 11/04/2014 05:17 PM, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 09:34 -0400, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 12:21 +0100, Petr Viktorin wrote: On 10/29/2014 10:37 AM, Martin Kosek wrote: On 10/28/2014 09:59 PM, Nathaniel McCallum wrote: On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote: This patch gives the administrator variables to control the size of the authentication and synchronization windows for OTP tokens. https://fedorahosted.org/freeipa/ticket/4511 NOTE: There is one known issue with this patch which I don't know how to solve. This patch changes the schema in install/share/60ipaconfig.ldif. On an upgrade, all of the new attributeTypes appear correctly. However, the modifications to the pre-existing objectClass do not show up on the server. What am I doing wrong? After modifying ipaGuiConfig manually, everything in this patch works just fine. This new version takes into account the new (proper) OIDs and attribute names. Thanks Nathaniel! The above known issue still remains. Petr3, any idea what could have gone wrong? ObjectClass MAY list extension should work just fine, AFAIK. You added a blank line to the LDIF file. This is an entry separator, so the objectClasses after the blank line don't belong to cn=schema, so they aren't considered in the update. Without the blank line it works fine. Thanks for the catch! Here is a version without the blank line. I forgot to remove the old steps defines. This patch performs this cleanup. I am now wondering, is the global config object really the nest place to add these OTP specific settings? I would prefer not to overload the object and instead: - create new ipaOTPConfig objectclass - add it to cn=otp,$SUFFIX - create otpconfig-mod and otpconfig-show commands to follow an example of dnsconfig-* and trustconfig-* commands IMO, this would allow more flexibility for the OTP settings and would also scale better for the future updates. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On 11/04/2014 05:17 PM, Nathaniel McCallum wrote:
On Wed, 2014-10-29 at 09:34 -0400, Nathaniel McCallum wrote:
On Wed, 2014-10-29 at 12:21 +0100, Petr Viktorin wrote:
On 10/29/2014 10:37 AM, Martin Kosek wrote:
On 10/28/2014 09:59 PM, Nathaniel McCallum wrote:
On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote:
This patch gives the administrator variables to control the size of
the authentication and synchronization windows for OTP tokens.
https://fedorahosted.org/freeipa/ticket/4511
NOTE: There is one known issue with this patch which I don't know how to
solve. This patch changes the schema in install/share/60ipaconfig.ldif.
On an upgrade, all of the new attributeTypes appear correctly. However,
the modifications to the pre-existing objectClass do not show up on the
server. What am I doing wrong?
After modifying ipaGuiConfig manually, everything in this patch works
just fine.
This new version takes into account the new (proper) OIDs and attribute
names.
Thanks Nathaniel!
The above known issue still remains.
Petr3, any idea what could have gone wrong? ObjectClass MAY list extension
should work just fine, AFAIK.
You added a blank line to the LDIF file. This is an entry separator, so
the objectClasses after the blank line don't belong to cn=schema, so
they aren't considered in the update.
Without the blank line it works fine.
Thanks for the catch!
Here is a version without the blank line.
I forgot to remove the old steps defines. This patch performs this
cleanup.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Hello Nathaniel,
Few comments on the review:
* in authcfg
o in string_to_types, I would prefer you set the last element
of 'map' to { NULL, 0 }.
o in entry_to_window, you may declare the 'defaults' array as
'static const'
o Would use define for ipaUserAuthType,ipaHOTPAuthWindow,
ipaTOTPAuthWindow, ipaHOTPSyncWindow,ipaTOTPSyncWindow
that are present multiple times
o suffix_to_config: cfg is set (and returned) calling
entry_to_config(entry). Now the entry_to_config returns a
structure on the stack so it is not valid to access outside
of the entry_to_config
o authcfg_fini free the configs. config-cfg should have
been allocated and must be freed (be care that configs-cfg
may contains DEFAULTS)
o authcfg_get_auth_types:322 should it return 'gbl' or
AUTHCFG_AUTH_TYPE_PASSWORD
o authcfg_get_auth_window/authcfg_get_sync_window returns a
window structure that is on the stack. It is not valid
outside of those functions
thanks
thierry
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On 7.11.2014 08:58, Martin Kosek wrote: On 11/04/2014 05:17 PM, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 09:34 -0400, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 12:21 +0100, Petr Viktorin wrote: On 10/29/2014 10:37 AM, Martin Kosek wrote: On 10/28/2014 09:59 PM, Nathaniel McCallum wrote: On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote: This patch gives the administrator variables to control the size of the authentication and synchronization windows for OTP tokens. https://fedorahosted.org/freeipa/ticket/4511 NOTE: There is one known issue with this patch which I don't know how to solve. This patch changes the schema in install/share/60ipaconfig.ldif. On an upgrade, all of the new attributeTypes appear correctly. However, the modifications to the pre-existing objectClass do not show up on the server. What am I doing wrong? After modifying ipaGuiConfig manually, everything in this patch works just fine. This new version takes into account the new (proper) OIDs and attribute names. Thanks Nathaniel! The above known issue still remains. Petr3, any idea what could have gone wrong? ObjectClass MAY list extension should work just fine, AFAIK. You added a blank line to the LDIF file. This is an entry separator, so the objectClasses after the blank line don't belong to cn=schema, so they aren't considered in the update. Without the blank line it works fine. Thanks for the catch! Here is a version without the blank line. I forgot to remove the old steps defines. This patch performs this cleanup. I am now wondering, is the global config object really the nest place to add these OTP specific settings? I would prefer not to overload the object and instead: - create new ipaOTPConfig objectclass - add it to cn=otp,$SUFFIX - create otpconfig-mod and otpconfig-show commands to follow an example of dnsconfig-* and trustconfig-* commands IMO, this would allow more flexibility for the OTP settings and would also scale better for the future updates. +1 I will comment the patch as if ^^ would not exist because it will still be needed in the new plugin. Because of ^^ I did not test, just read. 1. Got: install/ui/src/freeipa/serverconfig.js(135): lint warning: extra comma is not recommended in array initializers Please run: jsl -nofilelisting -nosummary -nologo -conf jsl.conf in install/ui directory The goal is no have no warnings and errors. 2. new attrs should be added to 'System: Read Global Configuration' managed permission -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On Fri, 2014-11-07 at 15:02 +0100, thierry bordaz wrote:
On 11/04/2014 05:17 PM, Nathaniel McCallum wrote:
On Wed, 2014-10-29 at 09:34 -0400, Nathaniel McCallum wrote:
On Wed, 2014-10-29 at 12:21 +0100, Petr Viktorin wrote:
On 10/29/2014 10:37 AM, Martin Kosek wrote:
On 10/28/2014 09:59 PM, Nathaniel McCallum wrote:
On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote:
This patch gives the administrator variables to control the size
of
the authentication and synchronization windows for OTP tokens.
https://fedorahosted.org/freeipa/ticket/4511
NOTE: There is one known issue with this patch which I don't know
how to
solve. This patch changes the schema in
install/share/60ipaconfig.ldif.
On an upgrade, all of the new attributeTypes appear correctly.
However,
the modifications to the pre-existing objectClass do not show up
on the
server. What am I doing wrong?
After modifying ipaGuiConfig manually, everything in this patch
works
just fine.
This new version takes into account the new (proper) OIDs and
attribute
names.
Thanks Nathaniel!
The above known issue still remains.
Petr3, any idea what could have gone wrong? ObjectClass MAY list
extension
should work just fine, AFAIK.
You added a blank line to the LDIF file. This is an entry separator, so
the objectClasses after the blank line don't belong to cn=schema, so
they aren't considered in the update.
Without the blank line it works fine.
Thanks for the catch!
Here is a version without the blank line.
I forgot to remove the old steps defines. This patch performs this
cleanup.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Hello Nathaniel,
Few comments on the review:
* in authcfg
* in string_to_types, I would prefer you set the
last element of 'map' to { NULL, 0 }.
Why? What I have is perfectly legal ISO C and is exactly the equivalent
of your code. In a structure initializer, undefined structure elements
are zero'd.
* in entry_to_window, you may declare the
'defaults' array as 'static const'
Fixed.
* Would use define for
ipaUserAuthType,ipaHOTPAuthWindow,
ipaTOTPAuthWindow,
ipaHOTPSyncWindow,ipaTOTPSyncWindow that
are present multiple times
Fixed.
* suffix_to_config: cfg is set (and returned)
calling entry_to_config(entry). Now the
entry_to_config returns a structure on the
stack so it is not valid to access outside of
the entry_to_config
Nope. We are not passing by reference but by copy. This is perfectly
valid C.
* authcfg_fini free the configs. config-cfg
should have been allocated and must be freed
(be care that configs-cfg may contains
DEFAULTS)
Nope. The config-cfg structure is allocated and freed when config is.
This is assignment by copy not by reference.
* authcfg_get_auth_types:322 should it return
'gbl' or AUTHCFG_AUTH_TYPE_PASSWORD
If both the global and per-user auth type is unset, the default is
AUTHCFG_AUTH_TYPE_PASSWORD. We special case this here so that we don't
have to special case it everywhere else in the code. The code is correct
as stands.
* authcfg_get_auth_window/authcfg_get_sync_window
returns a window structure that is on the stack. It is not valid outside of
those functions
Nope. Structure return by copy is perfectly legal ISO C.
From 7c348a5816b782b32f40ab00e7fd7cc6455f9600 Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum npmccal...@redhat.com
Date: Thu, 23 Oct 2014 15:18:26 -0400
Subject: [PATCH] Make token window sizes configurable
This patch gives the administrator variables to control the size of
the authentication and synchronization windows for OTP tokens.
https://fedorahosted.org/freeipa/ticket/4511
---
API.txt | 6 +-
VERSION | 4 +-
daemons/ipa-slapi-plugins/ipa-pwd-extop/authcfg.c | 200 +-
daemons/ipa-slapi-plugins/ipa-pwd-extop/authcfg.h | 17 ++
daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c | 79 +++--
daemons/ipa-slapi-plugins/ipa-pwd-extop/syncreq.c | 7 +-
daemons/ipa-slapi-plugins/libotp/libotp.c | 133 ++
daemons/ipa-slapi-plugins/libotp/libotp.h
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On 11/07/2014 05:40 PM, Nathaniel McCallum wrote:
On Fri, 2014-11-07 at 15:02 +0100, thierry bordaz wrote:
On 11/04/2014 05:17 PM, Nathaniel McCallum wrote:
On Wed, 2014-10-29 at 09:34 -0400, Nathaniel McCallum wrote:
On Wed, 2014-10-29 at 12:21 +0100, Petr Viktorin wrote:
On 10/29/2014 10:37 AM, Martin Kosek wrote:
On 10/28/2014 09:59 PM, Nathaniel McCallum wrote:
On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote:
This patch gives the administrator variables to control the size of
the authentication and synchronization windows for OTP tokens.
https://fedorahosted.org/freeipa/ticket/4511
NOTE: There is one known issue with this patch which I don't know how to
solve. This patch changes the schema in install/share/60ipaconfig.ldif.
On an upgrade, all of the new attributeTypes appear correctly. However,
the modifications to the pre-existing objectClass do not show up on the
server. What am I doing wrong?
After modifying ipaGuiConfig manually, everything in this patch works
just fine.
This new version takes into account the new (proper) OIDs and attribute
names.
Thanks Nathaniel!
The above known issue still remains.
Petr3, any idea what could have gone wrong? ObjectClass MAY list extension
should work just fine, AFAIK.
You added a blank line to the LDIF file. This is an entry separator, so
the objectClasses after the blank line don't belong to cn=schema, so
they aren't considered in the update.
Without the blank line it works fine.
Thanks for the catch!
Here is a version without the blank line.
I forgot to remove the old steps defines. This patch performs this
cleanup.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Hello Nathaniel,
Few comments on the review:
* in authcfg
* in string_to_types, I would prefer you set the
last element of 'map' to { NULL, 0 }.
Why? What I have is perfectly legal ISO C and is exactly the equivalent
of your code. In a structure initializer, undefined structure elements
are zero'd.
* in entry_to_window, you may declare the
'defaults' array as 'static const'
Fixed.
* Would use define for
ipaUserAuthType,ipaHOTPAuthWindow,
ipaTOTPAuthWindow,
ipaHOTPSyncWindow,ipaTOTPSyncWindow that
are present multiple times
Fixed.
* suffix_to_config: cfg is set (and returned)
calling entry_to_config(entry). Now the
entry_to_config returns a structure on the
stack so it is not valid to access outside of
the entry_to_config
Nope. We are not passing by reference but by copy. This is perfectly
valid C.
* authcfg_fini free the configs. config-cfg
should have been allocated and must be freed
(be care that configs-cfg may contains
DEFAULTS)
Nope. The config-cfg structure is allocated and freed when config is.
This is assignment by copy not by reference.
* authcfg_get_auth_types:322 should it return
'gbl' or AUTHCFG_AUTH_TYPE_PASSWORD
If both the global and per-user auth type is unset, the default is
AUTHCFG_AUTH_TYPE_PASSWORD. We special case this here so that we don't
have to special case it everywhere else in the code. The code is correct
as stands.
* authcfg_get_auth_window/authcfg_get_sync_window
returns a window structure that is on the stack. It is not valid outside of
those functions
Nope. Structure return by copy is perfectly legal ISO C.
Hi Nathaniel,
You are right, I am not use to structure assignment and all these
assignments are valid.
Sorry for the noise. The patch is fine for me.
thanks
theirry
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On Tue, 2014-11-04 at 11:17 -0500, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 09:34 -0400, Nathaniel McCallum wrote: On Wed, 2014-10-29 at 12:21 +0100, Petr Viktorin wrote: On 10/29/2014 10:37 AM, Martin Kosek wrote: On 10/28/2014 09:59 PM, Nathaniel McCallum wrote: On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote: This patch gives the administrator variables to control the size of the authentication and synchronization windows for OTP tokens. https://fedorahosted.org/freeipa/ticket/4511 NOTE: There is one known issue with this patch which I don't know how to solve. This patch changes the schema in install/share/60ipaconfig.ldif. On an upgrade, all of the new attributeTypes appear correctly. However, the modifications to the pre-existing objectClass do not show up on the server. What am I doing wrong? After modifying ipaGuiConfig manually, everything in this patch works just fine. This new version takes into account the new (proper) OIDs and attribute names. Thanks Nathaniel! The above known issue still remains. Petr3, any idea what could have gone wrong? ObjectClass MAY list extension should work just fine, AFAIK. You added a blank line to the LDIF file. This is an entry separator, so the objectClasses after the blank line don't belong to cn=schema, so they aren't considered in the update. Without the blank line it works fine. Thanks for the catch! Here is a version without the blank line. I forgot to remove the old steps defines. This patch performs this cleanup. Can I get a review on this soon? I need to make other changes in these files for another bug and I'd prefer to not have to shuffle between patches. Nathaniel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On Wed, 2014-10-29 at 09:34 -0400, Nathaniel McCallum wrote:
On Wed, 2014-10-29 at 12:21 +0100, Petr Viktorin wrote:
On 10/29/2014 10:37 AM, Martin Kosek wrote:
On 10/28/2014 09:59 PM, Nathaniel McCallum wrote:
On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote:
This patch gives the administrator variables to control the size of
the authentication and synchronization windows for OTP tokens.
https://fedorahosted.org/freeipa/ticket/4511
NOTE: There is one known issue with this patch which I don't know how to
solve. This patch changes the schema in install/share/60ipaconfig.ldif.
On an upgrade, all of the new attributeTypes appear correctly. However,
the modifications to the pre-existing objectClass do not show up on the
server. What am I doing wrong?
After modifying ipaGuiConfig manually, everything in this patch works
just fine.
This new version takes into account the new (proper) OIDs and attribute
names.
Thanks Nathaniel!
The above known issue still remains.
Petr3, any idea what could have gone wrong? ObjectClass MAY list extension
should work just fine, AFAIK.
You added a blank line to the LDIF file. This is an entry separator, so
the objectClasses after the blank line don't belong to cn=schema, so
they aren't considered in the update.
Without the blank line it works fine.
Thanks for the catch!
Here is a version without the blank line.
I forgot to remove the old steps defines. This patch performs this
cleanup.
From 6007faa6fc86de5087ab8028febe162557ea46be Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum npmccal...@redhat.com
Date: Thu, 23 Oct 2014 15:18:26 -0400
Subject: [PATCH] Make token window sizes configurable
This patch gives the administrator variables to control the size of
the authentication and synchronization windows for OTP tokens.
https://fedorahosted.org/freeipa/ticket/4511
---
API.txt | 6 +-
VERSION | 4 +-
daemons/ipa-slapi-plugins/ipa-pwd-extop/authcfg.c | 195 +-
daemons/ipa-slapi-plugins/ipa-pwd-extop/authcfg.h | 17 ++
daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c | 79 +++--
daemons/ipa-slapi-plugins/ipa-pwd-extop/syncreq.c | 7 +-
daemons/ipa-slapi-plugins/libotp/libotp.c | 133 +++
daemons/ipa-slapi-plugins/libotp/libotp.h | 30 ++--
install/share/60ipaconfig.ldif| 6 +-
install/ui/src/freeipa/serverconfig.js| 10 ++
install/ui/test/data/ipa_init.json| 3 +-
install/updates/40-otp.update | 6 +
ipalib/plugins/config.py | 31 +++-
ipalib/plugins/internal.py| 1 +
14 files changed, 333 insertions(+), 195 deletions(-)
diff --git a/API.txt b/API.txt
index 491d7a76fd1d2d50208d314d1600839ce295..4f204d0fa2e33dc4c9202645e111c25d2a545d70 100644
--- a/API.txt
+++ b/API.txt
@@ -514,7 +514,7 @@ args: 0,1,1
option: Str('version?', exclude='webui')
output: Output('result', None, None)
command: config_mod
-args: 0,25,3
+args: 0,29,3
option: Str('addattr*', cli_name='addattr', exclude='webui')
option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
option: Str('delattr*', cli_name='delattr', exclude='webui')
@@ -525,6 +525,8 @@ option: Str('ipadefaultprimarygroup', attribute=True, autofill=False, cli_name='
option: Str('ipagroupobjectclasses', attribute=True, autofill=False, cli_name='groupobjectclasses', csv=True, multivalue=True, required=False)
option: IA5Str('ipagroupsearchfields', attribute=True, autofill=False, cli_name='groupsearch', multivalue=False, required=False)
option: IA5Str('ipahomesrootdir', attribute=True, autofill=False, cli_name='homedirectory', multivalue=False, required=False)
+option: Int('ipahotpauthwindow', attribute=True, autofill=False, cli_name='hotp_auth_window', maxvalue=1000, minvalue=1, multivalue=False, required=False)
+option: Int('ipahotpsyncwindow', attribute=True, autofill=False, cli_name='hotp_sync_window', maxvalue=1000, minvalue=1, multivalue=False, required=False)
option: StrEnum('ipakrbauthzdata', attribute=True, autofill=False, cli_name='pac_type', csv=True, multivalue=True, required=False, values=(u'MS-PAC', u'PAD', u'nfs:NONE'))
option: Int('ipamaxusernamelength', attribute=True, autofill=False, cli_name='maxusername', minvalue=1, multivalue=False, required=False)
option: Bool('ipamigrationenabled', attribute=True, autofill=False, cli_name='enable_migration', multivalue=False, required=False)
@@ -533,6 +535,8 @@ option: Int('ipasearchrecordslimit', attribute=True, autofill=False, cli_name='s
option: Int('ipasearchtimelimit', attribute=True, autofill=False, cli_name='searchtimelimit', minvalue=-1, multivalue=False, required=False)
option: Str('ipaselinuxusermapdefault', attribute=True, autofill=False,
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On 10/28/2014 09:59 PM, Nathaniel McCallum wrote: On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote: This patch gives the administrator variables to control the size of the authentication and synchronization windows for OTP tokens. https://fedorahosted.org/freeipa/ticket/4511 NOTE: There is one known issue with this patch which I don't know how to solve. This patch changes the schema in install/share/60ipaconfig.ldif. On an upgrade, all of the new attributeTypes appear correctly. However, the modifications to the pre-existing objectClass do not show up on the server. What am I doing wrong? After modifying ipaGuiConfig manually, everything in this patch works just fine. This new version takes into account the new (proper) OIDs and attribute names. Thanks Nathaniel! The above known issue still remains. Petr3, any idea what could have gone wrong? ObjectClass MAY list extension should work just fine, AFAIK. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On 10/29/2014 10:37 AM, Martin Kosek wrote: On 10/28/2014 09:59 PM, Nathaniel McCallum wrote: On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote: This patch gives the administrator variables to control the size of the authentication and synchronization windows for OTP tokens. https://fedorahosted.org/freeipa/ticket/4511 NOTE: There is one known issue with this patch which I don't know how to solve. This patch changes the schema in install/share/60ipaconfig.ldif. On an upgrade, all of the new attributeTypes appear correctly. However, the modifications to the pre-existing objectClass do not show up on the server. What am I doing wrong? After modifying ipaGuiConfig manually, everything in this patch works just fine. This new version takes into account the new (proper) OIDs and attribute names. Thanks Nathaniel! The above known issue still remains. Petr3, any idea what could have gone wrong? ObjectClass MAY list extension should work just fine, AFAIK. You added a blank line to the LDIF file. This is an entry separator, so the objectClasses after the blank line don't belong to cn=schema, so they aren't considered in the update. Without the blank line it works fine. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On Wed, 2014-10-29 at 12:21 +0100, Petr Viktorin wrote:
On 10/29/2014 10:37 AM, Martin Kosek wrote:
On 10/28/2014 09:59 PM, Nathaniel McCallum wrote:
On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote:
This patch gives the administrator variables to control the size of
the authentication and synchronization windows for OTP tokens.
https://fedorahosted.org/freeipa/ticket/4511
NOTE: There is one known issue with this patch which I don't know how to
solve. This patch changes the schema in install/share/60ipaconfig.ldif.
On an upgrade, all of the new attributeTypes appear correctly. However,
the modifications to the pre-existing objectClass do not show up on the
server. What am I doing wrong?
After modifying ipaGuiConfig manually, everything in this patch works
just fine.
This new version takes into account the new (proper) OIDs and attribute
names.
Thanks Nathaniel!
The above known issue still remains.
Petr3, any idea what could have gone wrong? ObjectClass MAY list extension
should work just fine, AFAIK.
You added a blank line to the LDIF file. This is an entry separator, so
the objectClasses after the blank line don't belong to cn=schema, so
they aren't considered in the update.
Without the blank line it works fine.
Thanks for the catch!
Here is a version without the blank line.
From 6402e1f50885af226db35495063d8b50cf246300 Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum npmccal...@redhat.com
Date: Thu, 23 Oct 2014 15:18:26 -0400
Subject: [PATCH] Make token window sizes configurable
This patch gives the administrator variables to control the size of
the authentication and synchronization windows for OTP tokens.
https://fedorahosted.org/freeipa/ticket/4511
---
API.txt | 6 +-
VERSION | 4 +-
daemons/ipa-slapi-plugins/ipa-pwd-extop/authcfg.c | 195 +-
daemons/ipa-slapi-plugins/ipa-pwd-extop/authcfg.h | 17 ++
daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c | 77 +++--
daemons/ipa-slapi-plugins/ipa-pwd-extop/syncreq.c | 5 +-
daemons/ipa-slapi-plugins/libotp/libotp.c | 133 +++
daemons/ipa-slapi-plugins/libotp/libotp.h | 30 ++--
install/share/60ipaconfig.ldif| 6 +-
install/ui/src/freeipa/serverconfig.js| 10 ++
install/ui/test/data/ipa_init.json| 3 +-
install/updates/40-otp.update | 6 +
ipalib/plugins/config.py | 31 +++-
ipalib/plugins/internal.py| 1 +
14 files changed, 333 insertions(+), 191 deletions(-)
diff --git a/API.txt b/API.txt
index 491d7a76fd1d2d50208d314d1600839ce295..4f204d0fa2e33dc4c9202645e111c25d2a545d70 100644
--- a/API.txt
+++ b/API.txt
@@ -514,7 +514,7 @@ args: 0,1,1
option: Str('version?', exclude='webui')
output: Output('result', None, None)
command: config_mod
-args: 0,25,3
+args: 0,29,3
option: Str('addattr*', cli_name='addattr', exclude='webui')
option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
option: Str('delattr*', cli_name='delattr', exclude='webui')
@@ -525,6 +525,8 @@ option: Str('ipadefaultprimarygroup', attribute=True, autofill=False, cli_name='
option: Str('ipagroupobjectclasses', attribute=True, autofill=False, cli_name='groupobjectclasses', csv=True, multivalue=True, required=False)
option: IA5Str('ipagroupsearchfields', attribute=True, autofill=False, cli_name='groupsearch', multivalue=False, required=False)
option: IA5Str('ipahomesrootdir', attribute=True, autofill=False, cli_name='homedirectory', multivalue=False, required=False)
+option: Int('ipahotpauthwindow', attribute=True, autofill=False, cli_name='hotp_auth_window', maxvalue=1000, minvalue=1, multivalue=False, required=False)
+option: Int('ipahotpsyncwindow', attribute=True, autofill=False, cli_name='hotp_sync_window', maxvalue=1000, minvalue=1, multivalue=False, required=False)
option: StrEnum('ipakrbauthzdata', attribute=True, autofill=False, cli_name='pac_type', csv=True, multivalue=True, required=False, values=(u'MS-PAC', u'PAD', u'nfs:NONE'))
option: Int('ipamaxusernamelength', attribute=True, autofill=False, cli_name='maxusername', minvalue=1, multivalue=False, required=False)
option: Bool('ipamigrationenabled', attribute=True, autofill=False, cli_name='enable_migration', multivalue=False, required=False)
@@ -533,6 +535,8 @@ option: Int('ipasearchrecordslimit', attribute=True, autofill=False, cli_name='s
option: Int('ipasearchtimelimit', attribute=True, autofill=False, cli_name='searchtimelimit', minvalue=-1, multivalue=False, required=False)
option: Str('ipaselinuxusermapdefault', attribute=True, autofill=False, cli_name='ipaselinuxusermapdefault', multivalue=False, required=False)
option: Str('ipaselinuxusermaporder', attribute=True, autofill=False, cli_name='ipaselinuxusermaporder', multivalue=False,
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote:
This patch gives the administrator variables to control the size of
the authentication and synchronization windows for OTP tokens.
https://fedorahosted.org/freeipa/ticket/4511
NOTE: There is one known issue with this patch which I don't know how to
solve. This patch changes the schema in install/share/60ipaconfig.ldif.
On an upgrade, all of the new attributeTypes appear correctly. However,
the modifications to the pre-existing objectClass do not show up on the
server. What am I doing wrong?
After modifying ipaGuiConfig manually, everything in this patch works
just fine.
This new version takes into account the new (proper) OIDs and attribute
names. The above known issue still remains.
Nathaniel
From 70c85c066316acb7b15739c608c90ba1c0c38cbc Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum npmccal...@redhat.com
Date: Thu, 23 Oct 2014 15:18:26 -0400
Subject: [PATCH] Make token window sizes configurable
This patch gives the administrator variables to control the size of
the authentication and synchronization windows for OTP tokens.
https://fedorahosted.org/freeipa/ticket/4511
---
API.txt | 6 +-
VERSION | 4 +-
daemons/ipa-slapi-plugins/ipa-pwd-extop/authcfg.c | 195 +-
daemons/ipa-slapi-plugins/ipa-pwd-extop/authcfg.h | 17 ++
daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c | 77 +++--
daemons/ipa-slapi-plugins/ipa-pwd-extop/syncreq.c | 5 +-
daemons/ipa-slapi-plugins/libotp/libotp.c | 133 +++
daemons/ipa-slapi-plugins/libotp/libotp.h | 30 ++--
install/share/60ipaconfig.ldif| 7 +-
install/ui/src/freeipa/serverconfig.js| 10 ++
install/ui/test/data/ipa_init.json| 3 +-
install/updates/40-otp.update | 6 +
ipalib/plugins/config.py | 31 +++-
ipalib/plugins/internal.py| 1 +
14 files changed, 334 insertions(+), 191 deletions(-)
diff --git a/API.txt b/API.txt
index 491d7a76fd1d2d50208d314d1600839ce295..4f204d0fa2e33dc4c9202645e111c25d2a545d70 100644
--- a/API.txt
+++ b/API.txt
@@ -514,7 +514,7 @@ args: 0,1,1
option: Str('version?', exclude='webui')
output: Output('result', None, None)
command: config_mod
-args: 0,25,3
+args: 0,29,3
option: Str('addattr*', cli_name='addattr', exclude='webui')
option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
option: Str('delattr*', cli_name='delattr', exclude='webui')
@@ -525,6 +525,8 @@ option: Str('ipadefaultprimarygroup', attribute=True, autofill=False, cli_name='
option: Str('ipagroupobjectclasses', attribute=True, autofill=False, cli_name='groupobjectclasses', csv=True, multivalue=True, required=False)
option: IA5Str('ipagroupsearchfields', attribute=True, autofill=False, cli_name='groupsearch', multivalue=False, required=False)
option: IA5Str('ipahomesrootdir', attribute=True, autofill=False, cli_name='homedirectory', multivalue=False, required=False)
+option: Int('ipahotpauthwindow', attribute=True, autofill=False, cli_name='hotp_auth_window', maxvalue=1000, minvalue=1, multivalue=False, required=False)
+option: Int('ipahotpsyncwindow', attribute=True, autofill=False, cli_name='hotp_sync_window', maxvalue=1000, minvalue=1, multivalue=False, required=False)
option: StrEnum('ipakrbauthzdata', attribute=True, autofill=False, cli_name='pac_type', csv=True, multivalue=True, required=False, values=(u'MS-PAC', u'PAD', u'nfs:NONE'))
option: Int('ipamaxusernamelength', attribute=True, autofill=False, cli_name='maxusername', minvalue=1, multivalue=False, required=False)
option: Bool('ipamigrationenabled', attribute=True, autofill=False, cli_name='enable_migration', multivalue=False, required=False)
@@ -533,6 +535,8 @@ option: Int('ipasearchrecordslimit', attribute=True, autofill=False, cli_name='s
option: Int('ipasearchtimelimit', attribute=True, autofill=False, cli_name='searchtimelimit', minvalue=-1, multivalue=False, required=False)
option: Str('ipaselinuxusermapdefault', attribute=True, autofill=False, cli_name='ipaselinuxusermapdefault', multivalue=False, required=False)
option: Str('ipaselinuxusermaporder', attribute=True, autofill=False, cli_name='ipaselinuxusermaporder', multivalue=False, required=False)
+option: Int('ipatotpauthwindow', attribute=True, autofill=False, cli_name='totp_auth_window', maxvalue=2678400, minvalue=30, multivalue=False, required=False)
+option: Int('ipatotpsyncwindow', attribute=True, autofill=False, cli_name='totp_sync_window', maxvalue=2678400, minvalue=30, multivalue=False, required=False)
option: StrEnum('ipauserauthtype', attribute=True, autofill=False, cli_name='user_auth_type', csv=True, multivalue=True, required=False, values=(u'password', u'radius', u'otp'))
option: Str('ipauserobjectclasses', attribute=True, autofill=False,
Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable
On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote: This patch gives the administrator variables to control the size of the authentication and synchronization windows for OTP tokens. https://fedorahosted.org/freeipa/ticket/4511 NOTE: There is one known issue with this patch which I don't know how to solve. This patch changes the schema in install/share/60ipaconfig.ldif. On an upgrade, all of the new attributeTypes appear correctly. However, the modifications to the pre-existing objectClass do not show up on the server. What am I doing wrong? After modifying ipaGuiConfig manually, everything in this patch works just fine. Also, I need an allocation of OIDs for the new attributes. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
