Re: [Freeipa-devel] [PATCH 0078] Enable QR code display by default in otptoken-add
On Fri, 14 Nov 2014 20:05:35 +0100 Petr Viktorin wrote: > On 11/14/2014 08:03 PM, Petr Viktorin wrote: > > On 11/14/2014 07:26 PM, Simo Sorce wrote: > >> On Fri, 14 Nov 2014 14:08:24 +0100 > >> Petr Viktorin wrote: > >> > >>> On 11/14/2014 01:18 PM, Petr Vobornik wrote: > >>> [...] > > > > Nope, defaults are filled in by the client. (And also on the > > server if they're still missing; it's part of the common > > validation.) > > IMHO this is quite unfortunate behavior which may also fail > horribly if there is a newer client and an older server -> > backwards compatibility is on API level, not CLI level. Defaults > should be filled by server, not a client. We should seriously > reconsider the design of our CLI. But that's for different, > future discussion. > >>> > >>> You can't use a newer client with an older server, you get a > >>> VersionError in that case. > >> > >> Does it break only for this command ? > >> Or in general. > > > > In general. It's been built into the framework since IPA 2.0 [0]. > > There have been four years of development assuming this > > compatibility scheme. > > I should clarify – this is only for the API, i.e. the `ipa` command. > Clients of the "ipa-client-install" sort don't use the API. I know they don't or it would be a disaster. However it is unreasonable to keep changing the API without any 2 way compatibility going forward. I expect it to be extremely common for an admin to have a much more recent desktop then the server being used. Having to jump through hoops to use the admin cli is not friendly. And we do not change the actual CLI that much, so it would be unexpected. We need to take seriously in consideration compatibility going both ways (of course new commands should just get "NotSupported" errors when used against old servers. But old commands should work, there is no good reason to break this kind of compatibility, it is just an artefact of botched versioning we did a few years ago and it is about time we seriously address this, also because once we make one of these APIs public and supported we cannot willy nilly break it, and we cannot force people to change their software if all works well except a version number being sent in and out. Individual interfaces need to be versioned, and one an interface is release it must no change (a new version need to be created if changes are needed). Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0078] Enable QR code display by default in otptoken-add
On 11/14/2014 08:03 PM, Petr Viktorin wrote: On 11/14/2014 07:26 PM, Simo Sorce wrote: On Fri, 14 Nov 2014 14:08:24 +0100 Petr Viktorin wrote: On 11/14/2014 01:18 PM, Petr Vobornik wrote: [...] Nope, defaults are filled in by the client. (And also on the server if they're still missing; it's part of the common validation.) IMHO this is quite unfortunate behavior which may also fail horribly if there is a newer client and an older server -> backwards compatibility is on API level, not CLI level. Defaults should be filled by server, not a client. We should seriously reconsider the design of our CLI. But that's for different, future discussion. You can't use a newer client with an older server, you get a VersionError in that case. Does it break only for this command ? Or in general. In general. It's been built into the framework since IPA 2.0 [0]. There have been four years of development assuming this compatibility scheme. I should clarify – this is only for the API, i.e. the `ipa` command. Clients of the "ipa-client-install" sort don't use the API. If a Fedora 21 client can't talk to a RHEL 6 server we have a huge problem that we need to fix *yesterday*. Then we have a huge task on our hands. [0] ticket https://fedorahosted.org/freeipa/ticket/584 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0078] Enable QR code display by default in otptoken-add
On 11/14/2014 07:26 PM, Simo Sorce wrote: On Fri, 14 Nov 2014 14:08:24 +0100 Petr Viktorin wrote: On 11/14/2014 01:18 PM, Petr Vobornik wrote: [...] Nope, defaults are filled in by the client. (And also on the server if they're still missing; it's part of the common validation.) IMHO this is quite unfortunate behavior which may also fail horribly if there is a newer client and an older server -> backwards compatibility is on API level, not CLI level. Defaults should be filled by server, not a client. We should seriously reconsider the design of our CLI. But that's for different, future discussion. You can't use a newer client with an older server, you get a VersionError in that case. Does it break only for this command ? Or in general. In general. It's been built into the framework since IPA 2.0 [0]. There have been four years of development assuming this compatibility scheme. If a Fedora 21 client can't talk to a RHEL 6 server we have a huge problem that we need to fix *yesterday*. Then we have a huge task on our hands. [0] ticket https://fedorahosted.org/freeipa/ticket/584 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0078] Enable QR code display by default in otptoken-add
On Fri, 14 Nov 2014 14:08:24 +0100 Petr Viktorin wrote: > On 11/14/2014 01:18 PM, Petr Vobornik wrote: > [...] > >> > >> Nope, defaults are filled in by the client. (And also on the > >> server if they're still missing; it's part of the common > >> validation.) > > > > IMHO this is quite unfortunate behavior which may also fail > > horribly if there is a newer client and an older server -> > > backwards compatibility is on API level, not CLI level. Defaults > > should be filled by server, not a client. We should seriously > > reconsider the design of our CLI. But that's for different, future > > discussion. > > You can't use a newer client with an older server, you get a > VersionError in that case. Does it break only for this command ? Or in general. If a Fedora 21 client can't talk to a RHEL 6 server we have a huge problem that we need to fix *yesterday*. > Feel free to file a ticket. But yes, redesigning the API is not > exactly a priority. > > > That's said and given the circumstances, it is easier and cleaner to > > return the --qrcode back as no_param now than to deal with potential > > future issues. > > What's the reason to break the CLI by making it no_param? > > -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0019] Prefer TCP connections to UDP in krb5 clients
On Fri, 2014-11-14 at 09:06 -0500, Simo Sorce wrote: > On Fri, 14 Nov 2014 10:32:23 +0100 > Jakub Hrozek wrote: > > > On Thu, Nov 13, 2014 at 02:40:28PM -0500, Simo Sorce wrote: > > > On Thu, 13 Nov 2014 14:20:14 -0500 > > > Nathaniel McCallum wrote: > > > > > > > On Thu, 2014-11-13 at 14:02 -0500, Nathaniel McCallum wrote: > > > > > On Fri, 2014-11-07 at 15:39 +0100, Martin Kosek wrote: > > > > > > On 11/07/2014 03:28 PM, Nathaniel McCallum wrote: > > > > > > > > > > > > > >> On Nov 7, 2014, at 9:21 AM, Martin Kosek > > > > > > >> wrote: > > > > > > >> > > > > > > >> On 11/07/2014 03:03 PM, Simo Sorce wrote: > > > > > > >>> On Fri, 07 Nov 2014 14:53:17 +0100 > > > > > > >>> Martin Kosek wrote: > > > > > > >>> > > > > > > On 11/07/2014 02:51 PM, Simo Sorce wrote: > > > > > > > On Fri, 07 Nov 2014 14:26:06 +0100 > > > > > > > Martin Kosek wrote: > > > > > > > > > > > > > >> On 11/07/2014 02:20 PM, Simo Sorce wrote: > > > > > > >>> On Fri, 07 Nov 2014 08:02:02 +0100 > > > > > > >>> Martin Kosek wrote: > > > > > > >>> > > > > > > On 11/07/2014 01:46 AM, Simo Sorce wrote: > > > > > > > On Thu, 06 Nov 2014 18:00:21 -0500 > > > > > > > Nathaniel McCallum wrote: > > > > > > > > > > > > > >> On Fri, 2013-10-04 at 06:12 -0400, Simo Sorce > > > > > > >> wrote: > > > > > > >>> > > > > > > >>> - Original Message - > > > > > > On 3.10.2013 23:43, Nathaniel McCallum wrote: > > > > > > > Patch attached. > > > > > > > > > > > > I'm curious - what is the purpose of this patch? > > > > > > To prevent 1 second timeouts and re-transmits > > > > > > when OTP is in place? > > > > > > > > > > > > What is the expected performance impact? Could > > > > > > it be configured for OTP separately - somehow? > > > > > > (I guess that it is not possible now ...) > > > > > > >>> > > > > > > >>> It benefits also communication of large packets > > > > > > >>> (when large MS-PAC or CAMMAC AD Data are > > > > > > >>> attached), so it is a better choice for IPA in > > > > > > >>> general. Especially given we have multiple KDC > > > > > > >>> processes configured we do not want clients > > > > > > >>> wasting KDC resources by making multiple > > > > > > >>> processes do the same operation. > > > > > > >> > > > > > > >> So apparently this patch never got reviewed over a > > > > > > >> year ago. > > > > > > >> > > > > > > >> It was related to a bug which was opened in SSSD. > > > > > > >> However, when it became clear we wanted to solve > > > > > > >> this in FreeIPA, the SSSD bug was closed but no > > > > > > >> corresponding FreeIPA bug was opened. The patch > > > > > > >> then fell through the cracks. > > > > > > > > > > > > Right - without an associated ticket tracking the > > > > > > patch, it is too easy to loose it unless the author > > > > > > prods people to review it. > > > > > > > > > > > > >> Without this patch, if OTP validation runs long we > > > > > > >> get retransmits and failures. > > > > > > >> > > > > > > >> One question I have is how to handle this for > > > > > > >> upgrades since (I think) this patch only handles > > > > > > >> new installs. > > > > > > >> > > > > > > >> Anyway, this patch is somewhat urgent now. So help > > > > > > >> is appreciated. > > > > > > >> > > > > > > >> I have attached a rebased version which has no > > > > > > >> other changes. > > > > > > >> > > > > > > >> Nathaniel > > > > > > > > > > > > > > I am not sure we can do much on updates, we do not > > > > > > > have a client-update tool, I would just document it > > > > > > > I guess. Otherwise we'd have to go back to sssd > > > > > > > which can inject additional values in krb5.conf, > > > > > > > however I am not sure it would be ok to set > > > > > > > something like this in the sssd's pubconf > > > > > > > includes ... > > > > > > > > > > > > Agreed, pubconf update does not sound right. > > > > > > > > > > > > However, we already update krb5.conf on client > > > > > > updates, in %post: > > > > > > > > > > > > %post client > > > > > > if [ $1 -gt 1 ] ; then > > > > > > # Has the client been configured? > > > > > > restore=0 > > > > > > test -f > > > > > > '/var/lib/ipa-client/sysrestore/sysrestore.index' && > > > > > > restore=$(wc -l > > > > > > '/var/lib/ipa-client/sysrestore/sysrestore.index' | > > > > >
Re: [Freeipa-devel] [PATCH 0078] Enable QR code display by default in otptoken-add
On Fri, 2014-11-14 at 17:20 +0100, Petr Viktorin wrote:
> On 11/14/2014 02:25 PM, Petr Vobornik wrote:
> > On 14.11.2014 14:08, Petr Viktorin wrote:
> >> On 11/14/2014 01:18 PM, Petr Vobornik wrote:
> >> [...]
>
> Nope, defaults are filled in by the client. (And also on the server if
> they're still missing; it's part of the common validation.)
> >>>
> >>> IMHO this is quite unfortunate behavior which may also fail horribly if
> >>> there is a newer client and an older server -> backwards compatibility
> >>> is on API level, not CLI level. Defaults should be filled by server, not
> >>> a client. We should seriously reconsider the design of our CLI. But
> >>> that's for different, future discussion.
> >>
> >> You can't use a newer client with an older server, you get a
> >> VersionError in that case.
> >
> > And that's bad because, IMHO, this case may be more common that a newer
> > server and an older client, e.g., RHEL 6.5 server and Fedora 21 client.
> >
> >>
> >> Feel free to file a ticket. But yes, redesigning the API is not exactly
> >> a priority.
> >>
> >>> That's said and given the circumstances, it is easier and cleaner to
> >>> return the --qrcode back as no_param now than to deal with potential
> >>> future issues.
> >>
> >> What's the reason to break the CLI by making it no_param?
> >
> > Sorry I meant, no_option, there is no no_param. Because it returns it
> > back to Nathaniel's argument about interactive session and I agree with
> > it. Why would we have both --no-qrcode and --qrcode options available on
> > CLI?
>
> It breaks the CLI, and for that there should be a better reason than "it
> looks bad". The CLI is an interface as well.
The attached patch retains --qrcode but specified as no_option. It is
marked as deprecated.
From 51be0dcf3003a992909da935a0d2529b8768900e Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum
Date: Thu, 6 Nov 2014 15:30:13 -0500
Subject: [PATCH] Enable QR code display by default in otptoken-add
This is possible because python-qrcode's output now fits in a standard
terminal. Also, update ipa-otp-import and otptoken-add-yubikey to
disable QR code output as it doesn't make sense in these contexts.
https://fedorahosted.org/freeipa/ticket/4703
---
API.txt | 3 ++-
VERSION | 4 ++--
ipalib/plugins/otptoken.py | 5 +++--
ipalib/plugins/otptoken_yubikey.py | 1 +
ipaserver/install/ipa_otptoken_import.py | 2 +-
5 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/API.txt b/API.txt
index 491d7a76fd1d2d50208d314d1600839ce295..2a63f1e2349f0df69433fa7cb742e269cd42d79f 100644
--- a/API.txt
+++ b/API.txt
@@ -2592,7 +2592,7 @@ output: Entry('result', , Gettext('A dictionary representing an LDA
output: Output('summary', (, ), None)
output: PrimaryKey('value', None, None)
command: otptoken_add
-args: 1,22,3
+args: 1,23,3
arg: Str('ipatokenuniqueid', attribute=True, cli_name='id', multivalue=False, primary_key=True, required=False)
option: Str('addattr*', cli_name='addattr', exclude='webui')
option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
@@ -2611,6 +2611,7 @@ option: Int('ipatokentotpclockoffset', attribute=True, autofill=True, cli_name='
option: Int('ipatokentotptimestep', attribute=True, autofill=True, cli_name='interval', default=30, minvalue=5, multivalue=False, required=False)
option: Str('ipatokenvendor', attribute=True, cli_name='vendor', multivalue=False, required=False)
option: Flag('no_members', autofill=True, default=False, exclude='webui')
+option: Flag('no_qrcode', autofill=True, default=False)
option: Flag('qrcode?', autofill=True, default=False)
option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
option: Str('setattr*', cli_name='setattr', exclude='webui')
diff --git a/VERSION b/VERSION
index b0d41e5e1ec59ddefbdcccf588b97bac2ff798ee..bae782a4ec4333f8fdb610465a7b9ea3877c990e 100644
--- a/VERSION
+++ b/VERSION
@@ -90,5 +90,5 @@ IPA_DATA_VERSION=2010061412
# #
IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=108
-# Last change: pvoborni - manage authorization of keytab operations
+IPA_API_VERSION_MINOR=109
+# Last change: npmccallum - display qrcode by default
diff --git a/ipalib/plugins/otptoken.py b/ipalib/plugins/otptoken.py
index f48f0502992f1b5fed4f342cace1c404624b..f0850854f98e84e44acdcef311225220ac0129a3 100644
--- a/ipalib/plugins/otptoken.py
+++ b/ipalib/plugins/otptoken.py
@@ -268,7 +268,8 @@ class otptoken_add(LDAPCreate):
msg_summary = _('Added OTP token "%(value)s"')
takes_options = LDAPCreate.takes_options + (
-Flag('qrcode?', label=_('Display QR code')),
+Flag('qrcode?', label=_('(deprecated)'), flags=('no_option')),
+Flag('no_qrcode', label=_('Do not display QR code'), default=False),
)
has_
Re: [Freeipa-devel] FreeIPA integration with external DNS services
On 14.11.2014 02:22, Simo Sorce wrote: > On Tue, 11 Nov 2014 16:29:51 +0100 > Petr Spacek wrote: > >> Hello, >> >> this thread is about RFE >> "IPA servers when installed should register themselves in the >> external DNS" https://fedorahosted.org/freeipa/ticket/4424 >> >> It is not a complete design, just a raw idea. >> >> >> Use case >> >> FreeIPA installation to a network with existing DNS infrastructure + >> network administrator who is not willing to add/maintain new DNS >> servers "just for FreeIPA". >> >> >> High-level idea >> === >> - Transform dns* commands from FreeIPA framework to equivalent >> "nsupdate" commands and send DNS updates to existing DNS servers. >> - Provide necessary encryption/signing keys to nsupdate. >> >> >> 1) Integration to FreeIPA framework >> === >> First of all, we need to decide if "external DNS integration" can be >> used at the same time with FreeIPA-integrated DNS or not. >> Side-question is what to do if a first server is installed with >> external-DNS but another replica is being installed with >> integrated-DNS and so on. > > I think being able to integrate with an external DNS is important, and > not just at the server level, being able to distribute TSIG keys to > client would be nice too, though a lot less important, than getting > server integration.. Using TSIG on many clients is very problematic. Keep in mind that TSIG is basically HMAC computed using symmetric key so: a) Every client would have to use the same key - this is a security nightmare. b) We would have to distribute and maintain many keys for many^2 clients, which is an administrative nightmare. For *clients* I would rather stay with GSS-TSIG which is much more manageable because we can use Kerberos trust and Keytab distribution is already solved by ipa-client-install. Alternative for clients would be to use FreeIPA server as proxy which encapsulates TSIG keys and issues update requests on behalf of clients. This would be FreeIPA-specific but any TSIG-distribution mechanism will be FreeIPA-specific anyway. TSIG standard explicitly says that there is no standardized distribution mechanism. >> In other words, the question is if current "dns.py" plugin shipped >> with FreeIPA framework should be: >> >> a) Extended dns.py with dnsexternal-* commands >> -- >> Disadvantages: >> - It complicate FreeIPA DNS interface which is a complex beast even >> now. >> - We would have add condition to every DNS API call in installers >> which would increase horribleness of the installer code even more (or >> add another layer of abstraction...). > > I agree having a special command is undesirable. >> >> - I don't see a point in using integrated-DNS with external-DNS at >> the same time. To use integrated-DNS you have to get a proper DNS >> delegation from parent domain - and if you can get the delegation >> then there is no point in using external DNS ... > > I disagree on this point, it makes a lot of sense to have some zones > maintained by IPA and ... some not. > >> Advantages: >> - You can use external & integrated DNS at the same time. > > We can achieve the same w/o a new ipa level command. This needs to be decided by FreeIPA framework experts ... Petr^3 came up with clever 'proxy' idea for IPA-commands. This proxy would provide all ipa dns* commands and dispatch user-issued commands to appropriate FreeIPA-plugin (internal-dns or external-dns). This decision could depend on some values in LDAP. >> b) Replace dns.py with another implementation of current dnszone-* & >> dnsrecord-* API. >> - >> This seems like a cleaner approach to me. It could be shipped as >> ipa-server-dns-external package (opposed to "standard" ipa-server-dns >> package). >> >> Advantages: >> - It could seamlessly work with FreeIPA client installer because the >> dns*->nsupdate command transformation would be done on FreeIPA server >> and client doesn't need to know about it. >> - Does not require re-training/not much new documentation because >> commands are the same. >> >> Disadvantages: >> - You can't use integrated & external DNS at the same time (but I >> don't think it justifies the added complexity). > > I disagree. > > One of the reason to use a mix is to allow smooth migrations where > zones are moved into (or out of) IPA one by one. Good point, I agree. I will focus on supporting both (internal and external) DNS at the same time. >> Petr^3 or anyone else, what do you propose? > > I think what I'd like to see is to be able to configure a DNS zone in > LDAP and mark it external. > The zone would held the TSIG keys necessary to perform DNS updates. > > When the regular ipa dnsrecord-add etc... commands are called, the > framework detects that the zone is "external", fewttches the TSIG key > (if the user has access to it) and spawn an nsupdate command tha
Re: [Freeipa-devel] [PATCH 0078] Enable QR code display by default in otptoken-add
On 11/14/2014 02:25 PM, Petr Vobornik wrote: On 14.11.2014 14:08, Petr Viktorin wrote: On 11/14/2014 01:18 PM, Petr Vobornik wrote: [...] Nope, defaults are filled in by the client. (And also on the server if they're still missing; it's part of the common validation.) IMHO this is quite unfortunate behavior which may also fail horribly if there is a newer client and an older server -> backwards compatibility is on API level, not CLI level. Defaults should be filled by server, not a client. We should seriously reconsider the design of our CLI. But that's for different, future discussion. You can't use a newer client with an older server, you get a VersionError in that case. And that's bad because, IMHO, this case may be more common that a newer server and an older client, e.g., RHEL 6.5 server and Fedora 21 client. Feel free to file a ticket. But yes, redesigning the API is not exactly a priority. That's said and given the circumstances, it is easier and cleaner to return the --qrcode back as no_param now than to deal with potential future issues. What's the reason to break the CLI by making it no_param? Sorry I meant, no_option, there is no no_param. Because it returns it back to Nathaniel's argument about interactive session and I agree with it. Why would we have both --no-qrcode and --qrcode options available on CLI? It breaks the CLI, and for that there should be a better reason than "it looks bad". The CLI is an interface as well. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Add help string on how to configure multiple DNS forwards on cli
On 11/14/2014 01:38 PM, Martin Basti wrote: On 14/11/14 13:31, Thorsten Scherf wrote: On [Tue, 11.11.2014 16:50], Martin Basti wrote: On 11/11/14 15:42, Thorsten Scherf wrote: On [Tue, 11.11.2014 13:57], Martin Basti wrote: On 11/11/14 13:37, Thorsten Scherf wrote: The ipa-server-install man page is more descriptive on how to configure multiple DNS forwarders than the cli help. The cli help is more verbose now. https://fedorahosted.org//ticket/4465 Cheers, Thorsten Hello, thanks for patch, but I have a few objections. 1) Trac link in commit is corrupted 2) The forwarder option is in more installation scripts: * ipa-dns-install * ipa-replica-install * ipa-server-install Please find the new patch attached. https://fedorahosted.org/freeipa/ticket/4465 Cheers, Thorsten Thank you, ACK. Please read how to format a patch for future. http://www.freeipa.org/page/Contribute/Patch_Format yeah, sorry, will do the next time. Can you push the patch to master? Cheers, Thorsten Yes it can, but I don't have rights to do that. Pushed to: master: 4c670919a5b15b70ff6efb50e9bb60eb45cecdba ipa-4-1: 8a3389d30c8f47d58e889a9772d68f5e70a4cd71 Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0019] Prefer TCP connections to UDP in krb5 clients
On Fri, 14 Nov 2014 10:32:23 +0100
Jakub Hrozek wrote:
> On Thu, Nov 13, 2014 at 02:40:28PM -0500, Simo Sorce wrote:
> > On Thu, 13 Nov 2014 14:20:14 -0500
> > Nathaniel McCallum wrote:
> >
> > > On Thu, 2014-11-13 at 14:02 -0500, Nathaniel McCallum wrote:
> > > > On Fri, 2014-11-07 at 15:39 +0100, Martin Kosek wrote:
> > > > > On 11/07/2014 03:28 PM, Nathaniel McCallum wrote:
> > > > > >
> > > > > >> On Nov 7, 2014, at 9:21 AM, Martin Kosek
> > > > > >> wrote:
> > > > > >>
> > > > > >> On 11/07/2014 03:03 PM, Simo Sorce wrote:
> > > > > >>> On Fri, 07 Nov 2014 14:53:17 +0100
> > > > > >>> Martin Kosek wrote:
> > > > > >>>
> > > > > On 11/07/2014 02:51 PM, Simo Sorce wrote:
> > > > > > On Fri, 07 Nov 2014 14:26:06 +0100
> > > > > > Martin Kosek wrote:
> > > > > >
> > > > > >> On 11/07/2014 02:20 PM, Simo Sorce wrote:
> > > > > >>> On Fri, 07 Nov 2014 08:02:02 +0100
> > > > > >>> Martin Kosek wrote:
> > > > > >>>
> > > > > On 11/07/2014 01:46 AM, Simo Sorce wrote:
> > > > > > On Thu, 06 Nov 2014 18:00:21 -0500
> > > > > > Nathaniel McCallum wrote:
> > > > > >
> > > > > >> On Fri, 2013-10-04 at 06:12 -0400, Simo Sorce
> > > > > >> wrote:
> > > > > >>>
> > > > > >>> - Original Message -
> > > > > On 3.10.2013 23:43, Nathaniel McCallum wrote:
> > > > > > Patch attached.
> > > > >
> > > > > I'm curious - what is the purpose of this patch?
> > > > > To prevent 1 second timeouts and re-transmits
> > > > > when OTP is in place?
> > > > >
> > > > > What is the expected performance impact? Could
> > > > > it be configured for OTP separately - somehow?
> > > > > (I guess that it is not possible now ...)
> > > > > >>>
> > > > > >>> It benefits also communication of large packets
> > > > > >>> (when large MS-PAC or CAMMAC AD Data are
> > > > > >>> attached), so it is a better choice for IPA in
> > > > > >>> general. Especially given we have multiple KDC
> > > > > >>> processes configured we do not want clients
> > > > > >>> wasting KDC resources by making multiple
> > > > > >>> processes do the same operation.
> > > > > >>
> > > > > >> So apparently this patch never got reviewed over a
> > > > > >> year ago.
> > > > > >>
> > > > > >> It was related to a bug which was opened in SSSD.
> > > > > >> However, when it became clear we wanted to solve
> > > > > >> this in FreeIPA, the SSSD bug was closed but no
> > > > > >> corresponding FreeIPA bug was opened. The patch
> > > > > >> then fell through the cracks.
> > > > >
> > > > > Right - without an associated ticket tracking the
> > > > > patch, it is too easy to loose it unless the author
> > > > > prods people to review it.
> > > > >
> > > > > >> Without this patch, if OTP validation runs long we
> > > > > >> get retransmits and failures.
> > > > > >>
> > > > > >> One question I have is how to handle this for
> > > > > >> upgrades since (I think) this patch only handles
> > > > > >> new installs.
> > > > > >>
> > > > > >> Anyway, this patch is somewhat urgent now. So help
> > > > > >> is appreciated.
> > > > > >>
> > > > > >> I have attached a rebased version which has no
> > > > > >> other changes.
> > > > > >>
> > > > > >> Nathaniel
> > > > > >
> > > > > > I am not sure we can do much on updates, we do not
> > > > > > have a client-update tool, I would just document it
> > > > > > I guess. Otherwise we'd have to go back to sssd
> > > > > > which can inject additional values in krb5.conf,
> > > > > > however I am not sure it would be ok to set
> > > > > > something like this in the sssd's pubconf
> > > > > > includes ...
> > > > >
> > > > > Agreed, pubconf update does not sound right.
> > > > >
> > > > > However, we already update krb5.conf on client
> > > > > updates, in %post:
> > > > >
> > > > > %post client
> > > > > if [ $1 -gt 1 ] ; then
> > > > > # Has the client been configured?
> > > > > restore=0
> > > > > test -f
> > > > > '/var/lib/ipa-client/sysrestore/sysrestore.index' &&
> > > > > restore=$(wc -l
> > > > > '/var/lib/ipa-client/sysrestore/sysrestore.index' |
> > > > > awk '{print $1}')
> > > > >
> > > > > if [ -f '/etc/sssd/sssd.conf' -a $restore
> > > > > -ge 2 ]; then if ! grep -E -q
> > > > > '/var/lib/sss/pubconf/krb5.include.d/' /etc/krb5.conf
> > > > >
Re: [Freeipa-devel] [PATCH 0078] Enable QR code display by default in otptoken-add
On 14.11.2014 14:08, Petr Viktorin wrote: On 11/14/2014 01:18 PM, Petr Vobornik wrote: [...] Nope, defaults are filled in by the client. (And also on the server if they're still missing; it's part of the common validation.) IMHO this is quite unfortunate behavior which may also fail horribly if there is a newer client and an older server -> backwards compatibility is on API level, not CLI level. Defaults should be filled by server, not a client. We should seriously reconsider the design of our CLI. But that's for different, future discussion. You can't use a newer client with an older server, you get a VersionError in that case. And that's bad because, IMHO, this case may be more common that a newer server and an older client, e.g., RHEL 6.5 server and Fedora 21 client. Feel free to file a ticket. But yes, redesigning the API is not exactly a priority. That's said and given the circumstances, it is easier and cleaner to return the --qrcode back as no_param now than to deal with potential future issues. What's the reason to break the CLI by making it no_param? Sorry I meant, no_option, there is no no_param. Because it returns it back to Nathaniel's argument about interactive session and I agree with it. Why would we have both --no-qrcode and --qrcode options available on CLI? -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0078] Enable QR code display by default in otptoken-add
On 11/14/2014 01:18 PM, Petr Vobornik wrote: [...] Nope, defaults are filled in by the client. (And also on the server if they're still missing; it's part of the common validation.) IMHO this is quite unfortunate behavior which may also fail horribly if there is a newer client and an older server -> backwards compatibility is on API level, not CLI level. Defaults should be filled by server, not a client. We should seriously reconsider the design of our CLI. But that's for different, future discussion. You can't use a newer client with an older server, you get a VersionError in that case. Feel free to file a ticket. But yes, redesigning the API is not exactly a priority. That's said and given the circumstances, it is easier and cleaner to return the --qrcode back as no_param now than to deal with potential future issues. What's the reason to break the CLI by making it no_param? -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Add help string on how to configure multiple DNS forwards on cli
On 14/11/14 13:31, Thorsten Scherf wrote: On [Tue, 11.11.2014 16:50], Martin Basti wrote: On 11/11/14 15:42, Thorsten Scherf wrote: On [Tue, 11.11.2014 13:57], Martin Basti wrote: On 11/11/14 13:37, Thorsten Scherf wrote: The ipa-server-install man page is more descriptive on how to configure multiple DNS forwarders than the cli help. The cli help is more verbose now. https://fedorahosted.org//ticket/4465 Cheers, Thorsten Hello, thanks for patch, but I have a few objections. 1) Trac link in commit is corrupted 2) The forwarder option is in more installation scripts: * ipa-dns-install * ipa-replica-install * ipa-server-install Please find the new patch attached. https://fedorahosted.org/freeipa/ticket/4465 Cheers, Thorsten Thank you, ACK. Please read how to format a patch for future. http://www.freeipa.org/page/Contribute/Patch_Format yeah, sorry, will do the next time. Can you push the patch to master? Cheers, Thorsten Yes it can, but I don't have rights to do that. -- Martin Basti -- Martin Basti ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0078] Enable QR code display by default in otptoken-add
On 13.11.2014 19:12, Petr Viktorin wrote:
On 11/13/2014 06:02 PM, Nathaniel McCallum wrote:
On Thu, 2014-11-13 at 16:57 +0100, Petr Viktorin wrote:
On 11/13/2014 04:40 PM, Petr Vobornik wrote:
On 13.11.2014 16:19, Nathaniel McCallum wrote:
Like you, I like #2 the best. Attached is an implementation.
I like --no-qrcode as well.
Should we also keep qrcode as 'no_option' to maintain API
compatibility
(but not CLI)?
I don't think it is necessary. It only makes sense to specify --qrcode
in an interactive session.
Makes sense.
ACK
Not pushing yet to give time for NACK if anybody doesn't agree with the
API change.
Hold on, what is happening here?
Aren't all clients since 4.0 sending the qrcode option to the server?
We absolutely can not break backwards compatibility with released
versions.
We also should not break the CLI. Just make it a no-op option, and say
it's deprecated in the doc.
As I understand the current behavior, the qrcode option is *not* sent to
the server by default in any scenario.
Nope, defaults are filled in by the client. (And also on the server if
they're still missing; it's part of the common validation.)
IMHO this is quite unfortunate behavior which may also fail horribly if
there is a newer client and an older server -> backwards compatibility
is on API level, not CLI level. Defaults should be filled by server, not
a client. We should seriously reconsider the design of our CLI. But
that's for different, future discussion.
That's said and given the circumstances, it is easier and cleaner to
return the --qrcode back as no_param now than to deal with potential
future issues.
You can try it out, actually:
$ ipa -vv otptoken-add
ipa: INFO: trying https://vm-175.idm.lab.eng.brq.redhat.com/ipa/json
ipa: INFO: Forwarding 'otptoken_add' to json server
'https://vm-175.idm.lab.eng.brq.redhat.com/ipa/json'
ipa: INFO: Request: {
"id": 0,
"method": "otptoken_add",
"params": [
[
null
],
{
"all": false,
"ipatokenhotpcounter": 0,
"ipatokenotpalgorithm": "sha1",
"ipatokenotpdigits": 6,
"ipatokenotpkey":
"5\ufffdK\ufffd1\u000e\ufffd7,\ufffd_\ufffd\ufffd.0\ufffdM\ufffd\u0016\ufffd",
"ipatokentotpclockoffset": 0,
"ipatokentotptimestep": 30,
"no_members": false,
"qrcode": false,
"raw": false,
"type": "totp",
"version": "2.108"
}
]
}
ipa: INFO: Response: {
"error": null,
"id": 0,
"principal": "ad...@idm.lab.eng.brq.redhat.com",
...
--
Petr Vobornik
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0019] Prefer TCP connections to UDP in krb5 clients
On Thu, Nov 13, 2014 at 02:40:28PM -0500, Simo Sorce wrote:
> On Thu, 13 Nov 2014 14:20:14 -0500
> Nathaniel McCallum wrote:
>
> > On Thu, 2014-11-13 at 14:02 -0500, Nathaniel McCallum wrote:
> > > On Fri, 2014-11-07 at 15:39 +0100, Martin Kosek wrote:
> > > > On 11/07/2014 03:28 PM, Nathaniel McCallum wrote:
> > > > >
> > > > >> On Nov 7, 2014, at 9:21 AM, Martin Kosek
> > > > >> wrote:
> > > > >>
> > > > >> On 11/07/2014 03:03 PM, Simo Sorce wrote:
> > > > >>> On Fri, 07 Nov 2014 14:53:17 +0100
> > > > >>> Martin Kosek wrote:
> > > > >>>
> > > > On 11/07/2014 02:51 PM, Simo Sorce wrote:
> > > > > On Fri, 07 Nov 2014 14:26:06 +0100
> > > > > Martin Kosek wrote:
> > > > >
> > > > >> On 11/07/2014 02:20 PM, Simo Sorce wrote:
> > > > >>> On Fri, 07 Nov 2014 08:02:02 +0100
> > > > >>> Martin Kosek wrote:
> > > > >>>
> > > > On 11/07/2014 01:46 AM, Simo Sorce wrote:
> > > > > On Thu, 06 Nov 2014 18:00:21 -0500
> > > > > Nathaniel McCallum wrote:
> > > > >
> > > > >> On Fri, 2013-10-04 at 06:12 -0400, Simo Sorce wrote:
> > > > >>>
> > > > >>> - Original Message -
> > > > On 3.10.2013 23:43, Nathaniel McCallum wrote:
> > > > > Patch attached.
> > > >
> > > > I'm curious - what is the purpose of this patch? To
> > > > prevent 1 second timeouts and re-transmits when OTP
> > > > is in place?
> > > >
> > > > What is the expected performance impact? Could it be
> > > > configured for OTP separately - somehow? (I guess
> > > > that it is not possible now ...)
> > > > >>>
> > > > >>> It benefits also communication of large packets (when
> > > > >>> large MS-PAC or CAMMAC AD Data are attached), so it
> > > > >>> is a better choice for IPA in general. Especially
> > > > >>> given we have multiple KDC processes configured we do
> > > > >>> not want clients wasting KDC resources by making
> > > > >>> multiple processes do the same operation.
> > > > >>
> > > > >> So apparently this patch never got reviewed over a
> > > > >> year ago.
> > > > >>
> > > > >> It was related to a bug which was opened in SSSD.
> > > > >> However, when it became clear we wanted to solve this
> > > > >> in FreeIPA, the SSSD bug was closed but no
> > > > >> corresponding FreeIPA bug was opened. The patch then
> > > > >> fell through the cracks.
> > > >
> > > > Right - without an associated ticket tracking the patch,
> > > > it is too easy to loose it unless the author prods
> > > > people to review it.
> > > >
> > > > >> Without this patch, if OTP validation runs long we get
> > > > >> retransmits and failures.
> > > > >>
> > > > >> One question I have is how to handle this for upgrades
> > > > >> since (I think) this patch only handles new installs.
> > > > >>
> > > > >> Anyway, this patch is somewhat urgent now. So help is
> > > > >> appreciated.
> > > > >>
> > > > >> I have attached a rebased version which has no other
> > > > >> changes.
> > > > >>
> > > > >> Nathaniel
> > > > >
> > > > > I am not sure we can do much on updates, we do not have
> > > > > a client-update tool, I would just document it I guess.
> > > > > Otherwise we'd have to go back to sssd which can inject
> > > > > additional values in krb5.conf, however I am not sure
> > > > > it would be ok to set something like this in the sssd's
> > > > > pubconf includes ...
> > > >
> > > > Agreed, pubconf update does not sound right.
> > > >
> > > > However, we already update krb5.conf on client updates,
> > > > in %post:
> > > >
> > > > %post client
> > > > if [ $1 -gt 1 ] ; then
> > > > # Has the client been configured?
> > > > restore=0
> > > > test -f
> > > > '/var/lib/ipa-client/sysrestore/sysrestore.index' &&
> > > > restore=$(wc -l
> > > > '/var/lib/ipa-client/sysrestore/sysrestore.index' | awk
> > > > '{print $1}')
> > > >
> > > > if [ -f '/etc/sssd/sssd.conf' -a $restore -ge 2
> > > > ]; then if ! grep -E -q
> > > > '/var/lib/sss/pubconf/krb5.include.d/' /etc/krb5.conf
> > > > 2>/dev/null ; then
> > > > echo
> > > > "includedir /var/lib/sss/pubconf/krb5.include.d/"
> > > > > /etc/krb5.conf.ipanew cat /etc/krb5.conf
> > > > >>> /etc/krb5.conf.ipanew
> > > > mv /etc/krb5.conf.ipanew /etc/krb5.conf
> > > > /sbin/re
Re: [Freeipa-devel] [PATCHES] 0656-0673 Switch the test suite to pytest
On 10/29/2014 04:52 PM, Petr Viktorin wrote: On 10/29/2014 01:22 PM, Tomas Babej wrote: On 10/27/2014 04:38 PM, Petr Viktorin wrote: On 10/15/2014 02:58 PM, Petr Viktorin wrote: This almost completes the switch to pytest. There are two missing things: - the details of test results (--with-xunit) are not read correctly by Jenkins. I have a theory I'm investigating here. - the beakerlib integration is still not ready I'll not be available for the rest of the week so I'm sending this early, in case someone wants to take a look. I've updated (and rearranged) the patches after some more testing. Both points above are fixed. Individual plugins are broken out; some would be nice to even release independently of IPA. (There is some demand for the BeakerLib plugin; for that I'd only need to break the dependency on ipa_log_manager.) These depend on my patches 0656-0660. Thanks for this effort! Patch 0656: tests: Use PEP8-compliant setup/teardown method names There are some references to the old names in the test_ipapython and test_install: [tbabej@thinkpad7 ipatests]$ git grep setUpClass [tbabej@thinkpad7 ipatests]$ git grep tearDownClass [tbabej@thinkpad7 ipatests]$ git grep setUp test_install/test_updates.py:def setUp(self): test_ipapython/test_cookie.py:def setUp(self): test_ipapython/test_cookie.py:def setUp(self): test_ipapython/test_cookie.py:def setUp(self): test_ipapython/test_dn.py:def setUp(self): test_ipapython/test_dn.py:def setUp(self): test_ipapython/test_dn.py:def setUp(self): test_ipapython/test_dn.py:def setUp(self): test_ipapython/test_dn.py:def setUp(self): [tbabej@thinkpad7 ipatests]$ git grep tearDown test_install/test_updates.py:def tearDown(self): These are in unittest.testCase. It would be nice to convert those as well, but that's for a larger cleanup. Patch 0657: tests: Add configuration for pytest Shouldn't we rather change the class names? Ideally yes, but I don't think renaming most of our test classes would be worth the benefit. Patch 0658: ipatests.util.ClassChecker: Raise AttributeError in get_subcls ACK. Patch 0659: test_automount_plugin: Fix test ordering ACK. PATCH 0660: Use setup_class/teardown_class in Declarative tests --- a/ipatests/test_ipaserver/test_changepw.py +++ b/ipatests/test_ipaserver/test_changepw.py @@ -33,8 +33,7 @@ class test_changepw(XMLRPC_test, Unauthorized_HTTP_test): app_uri = '/ipa/session/change_password' -def setup(self): -super(test_changepw, self).setup() +def setup(cls): try: api.Command['user_add'](uid=testuser, givenname=u'Test', sn=u'User') api.Command['passwd'](testuser, password=u'old_password') @@ -43,12 +42,11 @@ def setup(self): 'Cannot set up test user: %s' % e ) -def teardown(self): +def teardown(cls): try: api.Command['user_del']([testuser]) except errors.NotFound: pass -super(test_changepw, self).teardown() The setup/teardown methods are not classmethods, so the name of the first argument should remain "self". Oops, thanks for the catch! PATCH 0661: dogtag plugin: Don't use doctest syntax for non-doctest examples ACK. PATCH 0662: test_webui: Don't use __init__ for test classes I don't think the following change will work: -def __init__(self, driver=None, config=None): +def setup(self, driver=None, config=None): self.request_timeout = 30 self.driver = driver self.config = config if not config: self.load_config() +self.get_driver().maximize_window() + +def teardown(self): +self.driver.quit() def load_config(self): """ @@ -161,20 +165,6 @@ def load_config(self): if 'type' not in c: c['type'] = DEFAULT_TYPE -def setup(self): -""" -Test setup -""" -if not self.driver: -self.driver = self.get_driver() -self.driver.maximize_window() - -def teardown(self): -""" -Test clean up -""" -self.driver.quit() The setup(self) method used to set the self.driver attribute on the class instance, now nothing sets it. The setup method should do: def setup(self, driver=None, config=None): self.request_timeout = 30 self.driver = driver self.config = config if not config: self.load_config() if not self.driver: self.driver = self.get_driver() self.driver.maximize_window() However,
