Re: [Freeipa-devel] [PATCH 0063] Update qrcode support for newer python-qrcode
On Wed, 2014-09-10 at 17:41 -0400, Nathaniel McCallum wrote: > This substantially reduces the FreeIPA dependencies and allows > QR codes to fit in a standard terminal. > > https://fedorahosted.org/freeipa/ticket/4430 A note about this patch: Quick review of this would be excellent. Even better would be someone volunteering to backport (patch should apply cleanly) this for Fedora 21 Alpha. This will substantially reduce the install media size for Fedora Server. A build-root override has already been requested for building against the new python-qrcode. Nathaniel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH 0063] Update qrcode support for newer python-qrcode
This substantially reduces the FreeIPA dependencies and allows
QR codes to fit in a standard terminal.
https://fedorahosted.org/freeipa/ticket/4430
From 95fddc3a24c5c22fc506ee11571da6c6c063243d Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum
Date: Wed, 10 Sep 2014 17:35:37 -0400
Subject: [PATCH] Update qrcode support for newer python-qrcode
This substantially reduces the FreeIPA dependencies and allows
QR codes to fit in a standard terminal.
https://fedorahosted.org/freeipa/ticket/4430
---
freeipa.spec.in| 4 ++--
ipalib/plugins/otptoken.py | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/freeipa.spec.in b/freeipa.spec.in
index a3f1010eff40ba7552ecd28e9831d1d08fc9a454..b672ecb03bdd73c1a911a6a982ccd894bebcbce4 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -59,7 +59,7 @@ BuildRequires: python-memcached
BuildRequires: sssd >= 1.9.2
BuildRequires: python-lxml
BuildRequires: python-pyasn1 >= 0.0.9a
-BuildRequires: python-qrcode
+BuildRequires: python-qrcode-core >= 5.0.0
BuildRequires: python-dns
BuildRequires: m2crypto
BuildRequires: check
@@ -248,7 +248,7 @@ Requires: python-nss >= 0.15
Requires: python-lxml
Requires: python-netaddr
Requires: libipa_hbac-python
-Requires: python-qrcode
+Requires: python-qrcode-core >= 5.0.0
Requires: python-pyasn1
Requires: python-dateutil
Requires: python-yubico
diff --git a/ipalib/plugins/otptoken.py b/ipalib/plugins/otptoken.py
index dfd010e7f8663b424d9c536907fcc93229181fa3..1bd85d4b952dc51ea800ed37c49b3c50aeb31492 100644
--- a/ipalib/plugins/otptoken.py
+++ b/ipalib/plugins/otptoken.py
@@ -246,7 +246,7 @@ class otptoken_add(LDAPCreate):
msg_summary = _('Added OTP token "%(value)s"')
takes_options = LDAPCreate.takes_options + (
-Flag('qrcode?', label=_('Display QR code (requires wide terminal)')),
+Flag('qrcode?', label=_('Display QR code')),
)
has_output_params = LDAPCreate.has_output_params + (
@@ -328,7 +328,7 @@ class otptoken_add(LDAPCreate):
qr = qrcode.QRCode()
qr.add_data(uri)
qr.make()
-qr.print_tty()
+qr.print_ascii(tty=True)
print "\n"
return rv
--
2.1.0
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 746-747 append domain into network.negotiate-auth.trusted-uris
On 9/10/2014 9:59 AM, Petr Vobornik wrote: On 4.9.2014 21:26, Endi Sukma Dewata wrote: On 8/29/2014 11:00 AM, Petr Vobornik wrote: [PATCH] 746 webui: append network.negotiate-auth.trusted-uris https://fedorahosted.org/freeipa/ticket/4478 Some comments/questions: 1. If I'm reading this correctly, if the preference is currently empty, the method will just return without setting the new value. Fixed. Yes, even if there was 'continue', it would be wrong. 2. If the new value already exists, the method will just return. Shouldn't it "continue" with the rest of the loop instead of "return"? This applies to #1 as well. Yes, fixed. 3. Using indexOf() to find a URI in a string can produce false matches. For example, aa.com will match baa.com. Ideally the existing value should be parsed into a collection of URI's, then the new URI should be matched using a proper URI matching algorithm. Fixed with function which matches splitted and striped values using strict equality. ACK. A possible improvement is to normalize the current value to remove excessive white spaces. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 720-729 OTP usability improvements
On 9/9/2014 10:48 AM, Petr Vobornik wrote:
[PATCH] 727 webui: hide empty fields and sections
Will the "counter" field strictly have a
value with HOTP only and "clock offset & interval" fields strictly have
value with TOTP only? Do these fields contain the configured values or
the effective values? I was just thinking maybe in the future some of
these fields might be configured empty and they will use the default
values instead. If that's not a problem then ACK.
Respective fields are present only in corresponding object classes ->
there won't be an HTOP token with 'clock offset'. If they are present,
they have a default value. -> No false hiding -> Shouldn't be a problem.
ACK.
What do you think about setting `hide_empty_widgets` global setting to
True?
On a read-only page I think it's OK to hide empty fields. But on edit
page I don't think we should do that by default.
Maybe we should first clarify what is a read-only page. All details
pages are writable if user has the rights or read-only if he doesn't.
But mostly it depends on individual fields/attributes.
Read-only page is a page that does not provide editable interface for
all fields displayed in the page. For example, the fields in the
certificate details page are never editable, so suppose it has empty
optional fields (e.g. cert extensions?) we probably can hide those
fields without confusing users.
Other details pages have read-only and edit modes, so it probably makes
more sense to keep the structure identical in both modes so optional
fields like 'Job Title' will always be visible. However, it's also
possible to hide the empty fields in the read-only mode if necessary to
clean up the display.
Does "empty" mean "unset", or "blank" like empty string/array? Does
"unset" always mean "non-editable and invisible" and "blank" always mean
"editable and visible but it's just empty"? If this definition isn't
strictly followed there could be an editable field that accidentally
gets hidden because it's empty.
My intent was to show everything what user can edit. And hide fields:
1. for which he doesn't have read rights
2. have no (undefined) or empty('') value
3. are explicitly hidden by other logic - not related to this patch
When thinking about #1. Maybe we should base it on a presence or rather
not presence of a specific objectclass, rather than on a value. That way
it would be less confusing for newcomers. The logic is: if fields is
bound to an object class and the class is not present (that also means
we don't have attribute level rights) -> hide it.
That would be hiding/displaying based on structural reason instead of
user rights. How about an attribute that exists but the user doesn't
have the read permission? I think we need to check both structural and
user rights.
#2 is questionable. IMHO it would require user tests. Also hiding on ''
value might not be always desirable. Nevertheless I like the behavior
but it may be caused by the fact that I already know what to expect.
Before we can make this behavior the default/global, how do we make sure
that optional fields such as 'Job Title' will always visible in the edit
page even if it's empty? I think to avoid unexpected behavior hiding
based on empty value should only be done in read-only page as I
mentioned above.
Generally if a field is supposed to be hidden in an edit page because of
other condition (e.g. token type), not because of the value, the code
should also use that condition instead of relying on the value being
unset. In this particular case, instead of:
if (field !== undefined) {
// display field
}
we probably should do:
var type = ...
if (type == 'hotp') {
// display hotp fields
} else { // totp
// display totp fields
}
I agree - use case #3.
This is actually hiding/displaying based on structural reason too. In
this case the structure is reflected in the (virtual) 'type' field.
4. If no "Owner" is specified when adding a token, it will default to
admin. Is this the intended behavior?
Sadly yes.
Maybe the adder dialog should show admin (or the current user?) as the
default owner instead of empty?
Note: all patches except for 727 are ACKed.
It's all ACKed now. Everything else can be handled separately.
--
Endi S. Dewata
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 746-747 append domain into network.negotiate-auth.trusted-uris
On 4.9.2014 21:26, Endi Sukma Dewata wrote:
On 8/29/2014 11:00 AM, Petr Vobornik wrote:
[PATCH] 746 webui: append network.negotiate-auth.trusted-uris
https://fedorahosted.org/freeipa/ticket/4478
Some comments/questions:
1. If I'm reading this correctly, if the preference is currently empty,
the method will just return without setting the new value.
Fixed. Yes, even if there was 'continue', it would be wrong.
2. If the new value already exists, the method will just return.
Shouldn't it "continue" with the rest of the loop instead of "return"?
This applies to #1 as well.
Yes, fixed.
3. Using indexOf() to find a URI in a string can produce false matches.
For example, aa.com will match baa.com. Ideally the existing value
should be parsed into a collection of URI's, then the new URI should be
matched using a proper URI matching algorithm.
Fixed with function which matches splitted and striped values using
strict equality.
[PATCH] 747 install: create ff krb extension on every install, replica
install and upgrade
We don't want to copy the extension from master to replica because the
replica may use newer version of FreeIPA and therefore the extension
code might be obsolete. Same reason for upgrades.
https://fedorahosted.org/freeipa/ticket/4478
ACK.
--
Petr Vobornik
From 6c6a5a8d519134c8ed31aaa68d1c99d0019e7137 Mon Sep 17 00:00:00 2001
From: Petr Vobornik
Date: Fri, 29 Aug 2014 13:18:20 +0200
Subject: [PATCH] webui: append network.negotiate-auth.trusted-uris
https://fedorahosted.org/freeipa/ticket/4478
---
install/ffextension/chrome/content/kerberosauth.js | 24 +-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/install/ffextension/chrome/content/kerberosauth.js b/install/ffextension/chrome/content/kerberosauth.js
index a79c925bf83f43e49abce88c5a556976c944236b..c5afde984bcf74b872bcc37ed762f89968ec81b2 100644
--- a/install/ffextension/chrome/content/kerberosauth.js
+++ b/install/ffextension/chrome/content/kerberosauth.js
@@ -42,7 +42,8 @@ var kerberosauth = {
referer: '2',
native_gss_lib: 'true',
trusted_uris: '',
-allow_proxies: 'true'
+allow_proxies: 'true',
+append: ['trusted_uris']
}
},
@@ -125,14 +126,25 @@ var kerberosauth = {
var self = this;
var prefs = Cc["@mozilla.org/preferences-service;1"].getService(Ci.nsIPrefBranch);
+var append_opts = options.append || [];
for (var opt in options) {
+if (!self.config_options[opt]) continue;
+
var name = self.config_options[opt][0];
var type = self.config_options[opt][1];
var value = options[opt];
if (type === 'str') {
+if (value && append_opts.indexOf(opt) > -1) {
+var current = prefs.getCharPref(name) || '';
+if (this.str_contains(current, value)) {
+continue;
+} else if (current) {
+value = current + ', ' + value;
+}
+}
prefs.setCharPref(name, value);
} else if (type ==='int') {
prefs.setIntPref(name, Number(value));
@@ -142,6 +154,16 @@ var kerberosauth = {
}
},
+str_contains: function(str, value) {
+
+if (!str) return false;
+var vals = str.split(',');
+for (var i=0, l=vals.length; i___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0014 Fix typo causing ipa-upgradeconfig to fail
Dne 10.9.2014 v 13:15 David Kupka napsal(a): https://fedorahosted.org/freeipa/ticket/4529 ACK. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 247-259] ID views - management part
On 09/10/2014 03:10 PM, Petr Viktorin wrote:
On 08/01/2014 12:30 PM, Tomas Babej wrote:
Hi,
the following set of patches implements the ID view creation and
management of views and ID overrides in IPA.
Pending questions:
1.) The patch 253 implements basic managed permissions for ID views and
ID overrides. Do we want to have a separate permission for assigning ID
views?
2.) Performance: idview-apply command takes ~100 seconds for hostgroups
with 1000 member hosts. I am able to lower that by 20-30% using raw ldap
calls, is bypassing the framework worth the performance gain? We'll lose
the possiblity to hook on exception callbacks.
Oh, missed this for some reason, sorry. But I replied inline.
I believe calling Commands from Commands is bad; if you need some common
functionality it should be put in a Python function.
Do exception callbacks help here in any way?
The commands also need more helpful documentation, I am working on that.
Not tested yet, but here are my comments on the patches:
0247:
The change to copy-schema-to-ca is not necessary. This is only used for
servers installed with Dogtag 9; for such installs new schema is added
online (to 99-user.ldif), so it will be replicated to the CA DS correctly.
Did you register the new OIDs?
0248-0249 look good
0250:
With so many names imported from one module, I find it more readable to
`from ipalib.plugins import baseldap`, and use qualified names later.
Same in the 2 other patches.
This is personal preference/tip, feel free to disagree :)
0521:
Could you also kill the backslash in _hostname_validator?
0252:
Typo in __doc__, should be "This functionality is primarily used"…
{idview,idoverride}.takes_params: flags needs to be a set -- here you've
specified 11 single-letter flags. (Don't we all love Python's iterable
strings.)
Only one `ipa help` topic will be created for idview and idoverride. Is
that intended?
The pattern_errmsg in idoverride's uid should be expanded to fully
describe the pattern.
0253 looks good
0254:
The DN refactoring is done; I don't think the asserts in pre_callbacks
are necessary any more.
0258:
idview_apply: typo in hostgroup doc, should be "hosts to apply the ID
view to" and "after running the idview-apply command".
Typos in output params, should be e.g. "this ID view"
idview_apply.execute: Is there a reason for calling idview_show instead
of get_dn_if_exists?
idview_apply.execute:
failed['hostgroup'].append((hostgroup, str(e))) -- str(e) doesn't
include the name of the exception, which is usually important
Calling a Command (here, host_mod) from another Command is, at the very
least, quite slow, with all the validation and detailed output. It's
also a debugging nightmare when things go wrong. And the _exc_wrapper is
an even worse debugging nightmare.
If you need some functionality in the host plugin that you need, put it
in a function and call that. Otherwise use ldap directly.
Do we have some precedent for the 'No host was affected.' message?
Consumers of the JSON API shouldn't need to cpecial-case this string.
By subclassing idview_unapply from idview_apply you're violating
Liskov's substitution principle. Make a common base class for them.
0259:
show_id_overrides: cn is a MUST attribute in ipa*Override; use
view.single_value['cn'] instead of get(). In enumerate_hosts, is None
okay if cn is missing?
--
Petr³
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 247-259] ID views - management part
On 08/01/2014 12:30 PM, Tomas Babej wrote:
Hi,
the following set of patches implements the ID view creation and
management of views and ID overrides in IPA.
Pending questions:
1.) The patch 253 implements basic managed permissions for ID views and
ID overrides. Do we want to have a separate permission for assigning ID
views?
2.) Performance: idview-apply command takes ~100 seconds for hostgroups
with 1000 member hosts. I am able to lower that by 20-30% using raw ldap
calls, is bypassing the framework worth the performance gain? We'll lose
the possiblity to hook on exception callbacks.
The commands also need more helpful documentation, I am working on that.
Not tested yet, but here are my comments on the patches:
0247:
The change to copy-schema-to-ca is not necessary. This is only used for
servers installed with Dogtag 9; for such installs new schema is added
online (to 99-user.ldif), so it will be replicated to the CA DS correctly.
Did you register the new OIDs?
0248-0249 look good
0250:
With so many names imported from one module, I find it more readable to
`from ipalib.plugins import baseldap`, and use qualified names later.
Same in the 2 other patches.
This is personal preference/tip, feel free to disagree :)
0521:
Could you also kill the backslash in _hostname_validator?
0252:
Typo in __doc__, should be "This functionality is primarily used"…
{idview,idoverride}.takes_params: flags needs to be a set -- here you've
specified 11 single-letter flags. (Don't we all love Python's iterable
strings.)
Only one `ipa help` topic will be created for idview and idoverride. Is
that intended?
The pattern_errmsg in idoverride's uid should be expanded to fully
describe the pattern.
0253 looks good
0254:
The DN refactoring is done; I don't think the asserts in pre_callbacks
are necessary any more.
0258:
idview_apply: typo in hostgroup doc, should be "hosts to apply the ID
view to" and "after running the idview-apply command".
Typos in output params, should be e.g. "this ID view"
idview_apply.execute: Is there a reason for calling idview_show instead
of get_dn_if_exists?
idview_apply.execute:
failed['hostgroup'].append((hostgroup, str(e))) -- str(e) doesn't
include the name of the exception, which is usually important
Calling a Command (here, host_mod) from another Command is, at the very
least, quite slow, with all the validation and detailed output. It's
also a debugging nightmare when things go wrong. And the _exc_wrapper is
an even worse debugging nightmare.
If you need some functionality in the host plugin that you need, put it
in a function and call that. Otherwise use ldap directly.
Do we have some precedent for the 'No host was affected.' message?
Consumers of the JSON API shouldn't need to cpecial-case this string.
By subclassing idview_unapply from idview_apply you're violating
Liskov's substitution principle. Make a common base class for them.
0259:
show_id_overrides: cn is a MUST attribute in ipa*Override; use
view.single_value['cn'] instead of get(). In enumerate_hosts, is None
okay if cn is missing?
--
Petr³
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 11 - re-enable uninstall option in ipa-kra-install
On 09/02/2014 05:05 AM, Ade Lee wrote: Re-enable uninstall feature for ipa-kra-install The underlying Dogtag issue (Dogtag ticket 1113) has been fixed. We can therefore re-enable the uninstall option for ipa-kra-install. Also, fixes an incorrect path in the ipa-pki-proxy.conf, and adds a debug statement to provide status to the user when an uninstall is done. Also, re-added the no_host_dns option which is used when unpacking a replica file. Part of the work for: https://fedorahosted.org/freeipa/ticket/3872 Please review. A new COPR Dogtag build with the relevant fix for ticket 1113 has been built. (pki-core-10.2.0-0.8.fc20) Please update your Dogtag build to this version. Thanks, Ade ACK To be pushed to master when fedorahosted comes back online. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 0014 Fix typo causing ipa-upgradeconfig to fail
https://fedorahosted.org/freeipa/ticket/4529
--
David Kupka
From cd0201fb906bcd7cd67c230a642e245f1b3f60a9 Mon Sep 17 00:00:00 2001
From: David Kupka
Date: Wed, 10 Sep 2014 11:57:46 +0200
Subject: [PATCH] Fix typo causing ipa-upgradeconfig to fail.
Replace 'post-certsave-command' by 'cert-postsave-command'.
https://fedorahosted.org/freeipa/ticket/4529
---
install/tools/ipa-upgradeconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/install/tools/ipa-upgradeconfig b/install/tools/ipa-upgradeconfig
index 5dbf3087b24c5420741e92fd7364d2165ae77bad..3914eb59066b515d33bebc19ca5afb4f50548bb2 100644
--- a/install/tools/ipa-upgradeconfig
+++ b/install/tools/ipa-upgradeconfig
@@ -713,7 +713,7 @@ def certificate_renewal_update(ca):
if pre_command != val:
break
-val = certmonger.get_request_value(request_id, 'post-certsave-command')
+val = certmonger.get_request_value(request_id, 'cert-postsave-command')
if val is not None:
val = val.split(' ', 1)[0]
val = os.path.basename(val)
--
1.9.3
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
