Re: [Freeipa-devel] [PATCH] 746-747 append domain into network.negotiate-auth.trusted-uris
On 10.9.2014 19:33, Endi Sukma Dewata wrote: 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. Pushed to: master: * 388a6432ed9c45ca84240c596f091268176d46f9 webui: append network.negotiate-auth.trusted-uris * 4e6a3c69b0e07e5ec32381a47281ad88e41aae5d install: create ff krb extension on every install, replica install and upgrade ipa-4-1: * de90d7d44930cfa615e50ed022de7ea48e31609a webui: append network.negotiate-auth.trusted-uris * 97aebf8635ba26398353c91de9ca60844bea8c27 install: create ff krb extension on every install, replica install and upgrade -- Petr Vobornik ___ 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 pvobo...@redhat.com
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; il; i++) {
+if (vals[i].trim() === value) return true;
+}
+return false;
+},
+
prompt: function(conf, options) {
var strs = Cc[@mozilla.org/intl/stringbundle;1].
getService(Ci.nsIStringBundleService).
--
1.9.3
___
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] 746-747 append domain into network.negotiate-auth.trusted-uris
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. 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. 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. [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. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
