Re: [Freeipa-devel] [PATCH 0128] ipalib/cli.py: pythonify Collector class

[Martin Babinsky]

On 01/28/2016 03:20 PM, Tomas Babej wrote:



On 01/27/2016 03:58 PM, Martin Babinsky wrote:

On 01/18/2016 06:43 PM, Martin Babinsky wrote:

A little patch that should make some future pylint errors disappear.




Attaching updated patch that does not promote direct molestation of
instance dictionaries.





Patch looks good, one thing I am concerened about though is that
__todict__ now returns a direct reference to the internal, mutable dict,
and no longer a (shallow) copy.

Maybe we should use dict.copy() there?

Tomas



Ah I didn't realize that. Fixed in updated patch.

--
Martin^3 Babinsky
From 81ab2ea956eda611717f24474b9c5f09cf4dcfff Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Mon, 18 Jan 2016 18:35:52 +0100
Subject: [PATCH] ipalib/cli.py: pythonify Collector class

The implementation of Collector class is hard for pylint to chew and unwieldy.
This patch rewrites the class to a more readable and pythonic form.
---
 ipalib/cli.py | 22 --
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/ipalib/cli.py b/ipalib/cli.py
index 136e0aeb8b876b2fe021f08e49a85a0fdeb4b21b..f4e173bc56c37088ba841369739bc2bd4e3e60a9 100644
--- a/ipalib/cli.py
+++ b/ipalib/cli.py
@@ -972,20 +972,30 @@ cli_application_commands = (
 
 class Collector(object):
 def __init__(self):
-object.__setattr__(self, '_Collector__options', {})
+self.__dict__.update(options={})
+self.options = {}  # silence pylint
 
 def __setattr__(self, name, value):
-if name in self.__options:
-v = self.__options[name]
+if name in dir(self):
+object.__setattr__(self, name, value)
+else:
+self.__setitem__(name, value)
+
+def __getitem__(self, item):
+return self.options[item]
+
+def __setitem__(self, key, value):
+if key in self.options:
+v = self.options[key]
 if type(v) is tuple:
 value = v + (value,)
 else:
 value = (v, value)
-self.__options[name] = value
-object.__setattr__(self, name, value)
+self.options[key] = value
 
 def __todict__(self):
-return dict(self.__options)
+return dict.copy(self.options)
+
 
 class CLIOptionParserFormatter(optparse.IndentedHelpFormatter):
 def format_argument(self, name, help_string):
-- 
2.5.0

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [PATCH] 951 webui: fail nicely if cookies are disabled

[Petr Vobornik]

Reworks also sessionStorage test because disablement of cookies might be 
connected with sessionStorage and localStorage. E.g. Chrome raises 
exception when *Storage is accessed with "Block sites from setting any 
data" settings set in "Content Settings/Cookies" section.


https://fedorahosted.org/freeipa/ticket/4338
--
Petr Vobornik

From b5ff7dfe9ba3a1f39b6e21019438c488dcc078b4 Mon Sep 17 00:00:00 2001
From: Petr Vobornik 
Date: Thu, 28 Jan 2016 16:07:06 +0100
Subject: [PATCH] webui: fail nicely if cookies are disabled

Reworks also sessionStorage test because disablement of cookies might be connected
with sessionStorage and localStorage. E.g. Chrome raises exception when *Storage
is accessed with "Block sites from setting any data" settings set in
"Content Settings/Cookies" section.

https://fedorahosted.org/freeipa/ticket/4338
---
 install/ui/src/freeipa/app_container.js | 26 --
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/install/ui/src/freeipa/app_container.js b/install/ui/src/freeipa/app_container.js
index 1194fd14263a1b6f1a5a0a2fa4bc605f344ef417..b6359742317ab6e8b955cbf6b8511a5b61c06893 100644
--- a/install/ui/src/freeipa/app_container.js
+++ b/install/ui/src/freeipa/app_container.js
@@ -63,14 +63,28 @@ define([
 
 phases.on('init', lang.hitch(this, function() {
 var deferred = new Deferred();
-if (!window.sessionStorage) {
-deferred.reject({
-message: "Web UI requires sessionStorage enabled. " +
- "This might be caused by too strict browser " +
- "configuration."
-});
+
+function reject(item) {
+var msg = "Web UI requires " + item + " enabled. " +
+  "Possible cause: too strict browser " +
+  "configuration.";
+deferred.reject({ message: msg });
 return deferred.promise;
 }
+
+function testStorage(storage) {
+try {
+if (!window[storage]) return false;
+} catch(e) {
+return false;
+}
+return true;
+}
+
+if (!window.navigator.cookieEnabled) return reject('cookies');
+if (!testStorage('sessionStorage')) return reject('sessionStorage');
+if (!testStorage('localStorage')) return reject('local  Storage');
+
 if (window.sessionStorage.getItem('logout')) {
 window.sessionStorage.removeItem('logout');
 var login_facet = reg.facet.get('login');
-- 
2.4.3

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0128] ipalib/cli.py: pythonify Collector class

[Tomas Babej]

On 01/28/2016 04:44 PM, Martin Babinsky wrote:
> On 01/28/2016 03:20 PM, Tomas Babej wrote:
>>
>>
>> On 01/27/2016 03:58 PM, Martin Babinsky wrote:
>>> On 01/18/2016 06:43 PM, Martin Babinsky wrote:
 A little patch that should make some future pylint errors disappear.



>>> Attaching updated patch that does not promote direct molestation of
>>> instance dictionaries.
>>>
>>>
>>>
>>
>> Patch looks good, one thing I am concerened about though is that
>> __todict__ now returns a direct reference to the internal, mutable dict,
>> and no longer a (shallow) copy.
>>
>> Maybe we should use dict.copy() there?
>>
>> Tomas
>>
> 
> Ah I didn't realize that. Fixed in updated patch.
> 

Nitpick: Sorry for being misleading - I did not mean to suggest invoking
the method using the dict type directly. While being equivalent, the

dict.copy(self.__options)

it's less idiomatic than:

self.__options.copy()

Tomas

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0128] ipalib/cli.py: pythonify Collector class

[Martin Babinsky]

On 01/28/2016 05:06 PM, Tomas Babej wrote:



On 01/28/2016 04:44 PM, Martin Babinsky wrote:

On 01/28/2016 03:20 PM, Tomas Babej wrote:



On 01/27/2016 03:58 PM, Martin Babinsky wrote:

On 01/18/2016 06:43 PM, Martin Babinsky wrote:

A little patch that should make some future pylint errors disappear.




Attaching updated patch that does not promote direct molestation of
instance dictionaries.





Patch looks good, one thing I am concerened about though is that
__todict__ now returns a direct reference to the internal, mutable dict,
and no longer a (shallow) copy.

Maybe we should use dict.copy() there?

Tomas



Ah I didn't realize that. Fixed in updated patch.



Nitpick: Sorry for being misleading - I did not mean to suggest invoking
the method using the dict type directly. While being equivalent, the

dict.copy(self.__options)

it's less idiomatic than:

self.__options.copy()

Tomas



Ah sorry I forgot how to python again.

Attaching patch.

--
Martin^3 Babinsky
From ab10808ca8124499f021f45dfef91fa64cbe495b Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Mon, 18 Jan 2016 18:35:52 +0100
Subject: [PATCH] ipalib/cli.py: pythonify Collector class

The implementation of Collector class is hard for pylint to chew and unwieldy.
This patch rewrites the class to a more readable and pythonic form.
---
 ipalib/cli.py | 22 --
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/ipalib/cli.py b/ipalib/cli.py
index 136e0aeb8b876b2fe021f08e49a85a0fdeb4b21b..55204169d7caa06615d7d4144bfe845acec75368 100644
--- a/ipalib/cli.py
+++ b/ipalib/cli.py
@@ -972,20 +972,30 @@ cli_application_commands = (
 
 class Collector(object):
 def __init__(self):
-object.__setattr__(self, '_Collector__options', {})
+self.__dict__.update(options={})
+self.options = {}  # silence pylint
 
 def __setattr__(self, name, value):
-if name in self.__options:
-v = self.__options[name]
+if name in dir(self):
+object.__setattr__(self, name, value)
+else:
+self.__setitem__(name, value)
+
+def __getitem__(self, item):
+return self.options[item]
+
+def __setitem__(self, key, value):
+if key in self.options:
+v = self.options[key]
 if type(v) is tuple:
 value = v + (value,)
 else:
 value = (v, value)
-self.__options[name] = value
-object.__setattr__(self, name, value)
+self.options[key] = value
 
 def __todict__(self):
-return dict(self.__options)
+return self.options.copy()
+
 
 class CLIOptionParserFormatter(optparse.IndentedHelpFormatter):
 def format_argument(self, name, help_string):
-- 
2.5.0

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 951 webui: fail nicely if cookies are disabled

[Tomas Babej]

On 01/28/2016 04:15 PM, Petr Vobornik wrote:
> Reworks also sessionStorage test because disablement of cookies might be
> connected with sessionStorage and localStorage. E.g. Chrome raises
> exception when *Storage is accessed with "Block sites from setting any
> data" settings set in "Content Settings/Cookies" section.
> 
> https://fedorahosted.org/freeipa/ticket/4338
> 
> 

Seems that two spaces inserted themselves to the error message for
localStorage :)

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 154] ipa-kdb: map_groups() consider all results

[Alexander Bokovoy]

On Tue, 05 Jan 2016, Sumit Bose wrote:

Hi,

to find out to which local group a external user is mapped we do a
dereference search over the external groups with the SIDs related to the
external user. If a SID is mapped to more than one external group we
currently consider only the first returned match. With this patch all
results are taken into account. This makes sure all expected local group
memberships are added to the PAC which resolves
https://fedorahosted.org/freeipa/ticket/5573.

The code looks fine but I have not tested it. Hopefully, will do so in
between FOSDEM and devconf.cz.


--
/ Alexander Bokovoy

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0011-0012][RFE] ipa-replica-manage: automatically clean dangling RUVs

[Stanislav Laznicka]

On 01/26/2016 06:56 PM, Martin Basti wrote:



On 25.01.2016 16:41, Stanislav Laznicka wrote:

Hi,

Worked those comments into the code. Also added a bit different info 
message in clean_ruv with ca=True (ipa-replica-manage:430).


Also adding stepst to reproduce:
1. Create a master and some replica (3 replicas is a good solution - 
1 with CA, 1 without, 1 to be dangling (with CA))

2. Change domain level to 0 and ipactl restart
3. Remove the "dangling-to-be" replica from masters.ipa.etc and from 
both ipaca and domain subtrees in mapping tree.config

4. Try to remove the dangling ruvs with the command

Cheers,
Standa


On 01/22/2016 01:22 PM, Martin Basti wrote:

Hello,

I have a few comments

PATCH Automatically detect and remove dangling RUVs

1)
+# get the Directory Manager password
+if options.dirman_passwd:
+dirman_passwd = options.dirman_passwd
+else:
+dirman_passwd = installutils.read_password('Directory Manager',
+confirm=False, validate=False, retry=False)
+if dirman_passwd is None:
+sys.exit('Directory Manager password is required')
+
+options.dirman_passwd = dirman_passwd

IMO you need only else branch here

if not options.dirman_password:
dirman_passwd = installutils.read_password('Directory Manager',
confirm=False, validate=False, retry=False)
if dirman_passwd is None:
sys.exit('Directory Manager password is required')
   options.dirman_passwd = dirman_passwd


2)
We should use new formatting in new code (more times in code)

+sys.exit(
+"Failed to get data from '%s' while trying to list 
replicas: %s" %

+(host, e)
+)

sys.exit(
"Failed to get data from '{host}' while trying to list 
replicas: {e}".format(

  host=host, e=e
)
)

3)
+# get all masters
+masters_dn = DN(('cn', 'masters'), ('cn', 'ipa'), ('cn', 
'etc'),

+ipautil.realm_to_suffix(realm))

IMO you should use constants:
 masters_dn = DN(api.env.container_masters, api.env.basedn)

4)
+# Get realm string for config tree
+s = realm.split('.')
+s = ['dc={dc},'.format(dc=x.lower()) for x in s]
+realm_config = DN(('cn', ''.join(s)[0:-1]))

Can be api.env.basedn used instead of this block of code?

5)
+masters = [x.single_value['cn'] for x in masters]

+for master in masters:

is there any reason why not iterate over the keys in info dict?

for master_name, master_data/values/whatever in info.items():
   master_data['online'] = True

Looks better than: info[master]['online'] = True

6)
I asked python gurus, for empty lists and dicts, please use [] and 
{} instead of list() and dict()

It is preferred and faster.

7)
+if(info[master]['ca']):
+entry = conn.get_entry(csreplica_dn)
+csruv = (master, 
entry.single_value.get('nsDS5ReplicaID'))

+if csruv not in csruvs:
+csruvs.append(csruv)

I dont like too much adding tuples into list and then doing search 
there, but it is as designed


However can you use set() instead of list when the purpose of 
variable is only testing existence?


related to:
csruvs
ruvs
offlines
clean_list
cleaned

8)
conn in finally block may be undefined

9)
unused local variables

clean_list
entry on line 570

10)
optional, comment what keys means in info structure




Hello,

1)
I accept your silence as the following code cannot use nothing from 
api.env

+# Get realm string for config tree
+s = realm.split('.')
+s = ['dc={dc},'.format(dc=x.lower()) for x in s]
+realm_config = DN(('cn', ''.join(s)[0:-1]))

but then please use:
s = ['dc={dc}'.format(dc=x.lower()) for x in s]
realm_config = DN(('cn', ','.join(s)))

But I still think that api.env.basedn can be used, because it contains 
the same format as you need

realm_config = DN(('cn', api.env.basedn))

Sorry, forgot to mention that in the last email. However, turns out you 
are right. I didn't think DN works like this but it does, so this is now 
in it.

2) nitpick
ca_dn = DN(('cn', 'ca'), DN(master.dn))

AFAIK can be just

ca_dn = DN(('cn', 'ca'), master.dn)


My bad, fixed.

3) uber nitpick
This is PEP8 valid, but somehow inconsistent with the rest of code and 
it hit my eyes


print('\t\tid: {id}, hostname: {host}'
.format(id=csruv[1], host=csruv[0])
)

we use in code

print(
   something1,
   something2
)

or

print(something1,
something2)


And that shouldn't be there anymore, too :)

Otherwise LGTM


From a1421841c88ab233179f175f49000995b2db4acc Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka 
Date: Fri, 18 Dec 2015 10:30:44 +0100
Subject: [PATCH 1/2] Listing and cleaning RUV extended for CA suffix

https://fedorahosted.org/freeipa/ticket/5411
---
 install/tools/ipa-replica-manage | 44 ++--
 ipaserver/install/replication.py |  2 +-
 

Re: [Freeipa-devel] [PATCH] 951 webui: fail nicely if cookies are disabled

[Petr Vobornik]

On 01/28/2016 04:23 PM, Tomas Babej wrote:



On 01/28/2016 04:15 PM, Petr Vobornik wrote:

Reworks also sessionStorage test because disablement of cookies might be
connected with sessionStorage and localStorage. E.g. Chrome raises
exception when *Storage is accessed with "Block sites from setting any
data" settings set in "Content Settings/Cookies" section.

https://fedorahosted.org/freeipa/ticket/4338




Seems that two spaces inserted themselves to the error message for
localStorage :)



updated patch attached.
--
Petr Vobornik
From 1e2306c426fef8ef2aa4e71fbcb197b49f13cdba Mon Sep 17 00:00:00 2001
From: Petr Vobornik 
Date: Thu, 28 Jan 2016 16:07:06 +0100
Subject: [PATCH] webui: fail nicely if cookies are disabled

Reworks also sessionStorage test because disablement of cookies might be connected
with sessionStorage and localStorage. E.g. Chrome raises exception when *Storage
is accessed with "Block sites from setting any data" settings set in
"Content Settings/Cookies" section.

https://fedorahosted.org/freeipa/ticket/4338
---
 install/ui/src/freeipa/app_container.js | 26 --
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/install/ui/src/freeipa/app_container.js b/install/ui/src/freeipa/app_container.js
index 1194fd14263a1b6f1a5a0a2fa4bc605f344ef417..bbe57ac97bd7b4877f9f90b6f4059771a26046ca 100644
--- a/install/ui/src/freeipa/app_container.js
+++ b/install/ui/src/freeipa/app_container.js
@@ -63,14 +63,28 @@ define([
 
 phases.on('init', lang.hitch(this, function() {
 var deferred = new Deferred();
-if (!window.sessionStorage) {
-deferred.reject({
-message: "Web UI requires sessionStorage enabled. " +
- "This might be caused by too strict browser " +
- "configuration."
-});
+
+function reject(item) {
+var msg = "Web UI requires " + item + " enabled. " +
+  "Possible cause: too strict browser " +
+  "configuration.";
+deferred.reject({ message: msg });
 return deferred.promise;
 }
+
+function testStorage(storage) {
+try {
+if (!window[storage]) return false;
+} catch(e) {
+return false;
+}
+return true;
+}
+
+if (!window.navigator.cookieEnabled) return reject('cookies');
+if (!testStorage('sessionStorage')) return reject('sessionStorage');
+if (!testStorage('localStorage')) return reject('localStorage');
+
 if (window.sessionStorage.getItem('logout')) {
 window.sessionStorage.removeItem('logout');
 var login_facet = reg.facet.get('login');
-- 
2.4.3

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 0003 webui: Issue New Certificate dialogs validates data

[Pavel Vomacka]

Hello,

On 01/28/2016 12:20 PM, Pavel Vomacka wrote:

'Issue new certificate' dialog now validates whether user fills 'principal' and 
'csr' field.
In case that one of these fields is empty then it does not allow to submit the 
dialog.

https://fedorahosted.org/freeipa/ticket/5432
I'm sending edited patch. One useless section is removed. The textarea 
for CSR is moved to another section which has the same layout. The 
method for creating content was useless, too. So it is removed in this 
patch.


--
Pavel Vomacka
>From 975ee191d3ccde82af30370a73970d10b52fb5aa Mon Sep 17 00:00:00 2001
From: Pavel Vomacka 
Date: Thu, 28 Jan 2016 17:37:29 +0100
Subject: [PATCH] Add validation to Issue new certificate dialog

'Issue new certificate' dialog now validates whether user fills 'principal' and 'csr' field.
In case that one of these fields is empty then it does not allow to submit the dialog.

https://fedorahosted.org/freeipa/ticket/5432
---
 install/ui/src/freeipa/certificate.js | 72 +--
 install/ui/src/freeipa/details.js |  5 +++
 2 files changed, 56 insertions(+), 21 deletions(-)

diff --git a/install/ui/src/freeipa/certificate.js b/install/ui/src/freeipa/certificate.js
index 93f3cfc68a95bfb8014aaf96d1b571568ac605dc..505a16fb8e031775989fcf141ffa7e65fd868472 100755
--- a/install/ui/src/freeipa/certificate.js
+++ b/install/ui/src/freeipa/certificate.js
@@ -30,10 +30,11 @@ define([
 './reg',
 './rpc',
 './text',
+'./widget',
 './dialog'],
 function(
 lang, builder, metadata_provider, IPA, $, menu,
-phases, reg, rpc, text) {
+phases, reg, rpc, text, widget_mod) {
 
 var exp = IPA.cert = {};
 
@@ -399,11 +400,35 @@ IPA.cert.request_dialog = function(spec) {
 spec = spec || {};
 
 spec.sections = spec.sections || [];
-var section = { fields: [] };
-spec.sections.push(section);
+var section0 = { fields: [] };
+	var section_csr = {
+		show_header: false,
+		fields: [
+			{
+field: false, // the section does not contain any field.
+$type: 'html',
+name: 'message_html_widget',
+html: spec.message
+			},
+			{
+$type: 'textarea',
+name: 'textarea_cert',
+required: true
+			}
+		],
+		layout:
+		{
+			$factory: widget_mod.fluid_layout,
+			widget_cls : "col-sm-12 controls",
+			label_cls : "hide"
+		}
+	};
+
+	spec.sections.push(section0);
+	spec.sections.push(section_csr);
 
 if (spec.show_principal) {
-section.fields.push(
+section0.fields.push(
 {
 $type: 'text',
 name: 'principal',
@@ -418,7 +443,7 @@ IPA.cert.request_dialog = function(spec) {
 }
 );
 }
-section.fields.push(
+section0.fields.push(
 {
 $type: 'entity_select',
 name: 'profile_id',
@@ -443,8 +468,26 @@ IPA.cert.request_dialog = function(spec) {
 click: function() {
 var values = {};
 that.save(values);
-var request = $.trim(that.textarea.val());
-values.request = IPA.cert.pem_csr_format(request);
+
+			// check requested fields
+			if (!that.validate()) {
+var dialog_fields = that.fields.get_fields();
+
+for (var i=0; i

Re: [Freeipa-devel] New tool tips for Refresh, Revert, Undo and Undo All buttons

[Petr Vobornik]

On 01/25/2016 01:55 PM, Pavel Vomacka wrote:

Hello everyone,

I just made a patch for the
https://fedorahosted.org/freeipa/ticket/5428 ticket. The patch adds
tool tips to the buttons in detail views. The text of new tool tips
is written in the comment of the ticket.

Pavel Vomacka Intern



Hi, there are few issues, NACK.

1. this patch uses tabs instead of spaces, previous was correct

2. code which focuses first invalid field could be replaced by:
  widget_mod.focus_invalid(that);

3. there is a convention that field and widget names uses the same name 
as the param, therefore 'textarea_cert' should be 'csr'. There is no 
convention for messages in html widget but it might be better to use a 
name reflecting purpose and not implementation. Instead of 
'message_html_widget' use 'instructions' or just 'message' - consistent 
with spec option. 	


Also I've filed: https://fedorahosted.org/freeipa/ticket/5652
--
Petr Vobornik

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [TEST][Patch 0020] Enabled recreation of test directory during ipa reinstallation

[Oleg Fayans]

Fellas, can anyone spend a free moment to review this?

On 01/27/2016 09:24 AM, Oleg Fayans wrote:
> Hi guys,
> 
> Any chance this can be reviewed any time soon?
> 
> On 01/21/2016 12:59 PM, Oleg Fayans wrote:
>>
>>
>>
> 

-- 
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0002] Refactor test_group_plugin

[Filip Skola]

- Original Message -
> On 01/25/2016 11:11 AM, Filip Skola wrote:
> >
> > - Original Message -
> >> On 01/15/2016 03:38 PM, Filip Skola wrote:
> >>> Hi,
> >>>
> >>> sending rebased patch.
> >>>
> >>> F.
> >>>
> >>> - Original Message -
>  Hello,
> 
>  sorry for delays. The patch no longer applies to master. Rebase it,
>  please.
> 
>  Milan
> 
>  - Original Message -
>  From: "Filip Škola" 
>  To: "Milan Kubík" 
>  Cc: freeipa-devel@redhat.com
>  Sent: Wednesday, 9 December, 2015 7:01:02 PM
>  Subject: Re: [Freeipa-devel] [PATCH 0002] Refactor test_group_plugin
> 
>  On Mon, 7 Dec 2015 17:49:18 +0100
>  Milan Kubík  wrote:
> 
> > On 12/03/2015 08:15 PM, Filip Škola wrote:
> >> On Mon, 30 Nov 2015 17:18:30 +0100
> >> Milan Kubík  wrote:
> >>
> >>> On 11/23/2015 04:42 PM, Filip Škola wrote:
>  Sending updated patch.
> 
>  F.
> 
>  On Mon, 23 Nov 2015 14:59:34 +0100
>  Filip Škola  wrote:
> 
> > Found couple of issues (broke some dependencies).
> >
> > NACK
> >
> > F.
> >
> > On Fri, 20 Nov 2015 13:56:36 +0100
> > Filip Škola  wrote:
> >
> >> Another one.
> >>
> >> F.
> >>> Hi, the tests look good. Few remarks, though.
> >>>
> >>> 1. Please, use the shortes copyright notice in new modules.
> >>>
> >>> #
> >>> # Copyright (C) 2015  FreeIPA Contributors see COPYING for
> >>> license #
> >>>
> >>> 2. The tests `test_group_remove_group_from_protected_group` and
> >>> `test_group_full_set_of_objectclass_not_available_post_detach`
> >>> were not ported. Please, include them in the patch.
> >>>
> >>> Also, for less hassle, please rebase your patches on top of
> >>> freeipa-mkubik-0025-3-Separated-Tracker-implementations-into-standalone-pa.patch
> >>> Which changes the location of tracker implementations and prevents
> >>> circular imports.
> >>>
> >>> Thanks.
> >>>
> >> Hi,
> >>
> >> these cases are there, in corresponding classes. They are marked
> >> with the original comments. (However I can move them to separate
> >> class if desirable.)
> >>
> >> The copyright notice is changed. Also included a few changes in the
> >> test with user without private group.
> >>
> >> Filip
> > NACK
> >
> > linter:
> > * Module tracker.group_plugin
> > ipatests/test_xmlrpc/tracker/group_plugin.py:257:
> > [E0102(function-redefined), GroupTracker.check_remove_member] method
> > already defined line 253)
> >
> > Probably a leftover after the rebase made on top of my patch. Please
> > fix it. You can check youch changes by make-lint script before
> > sending them.
> >
> > Thanks
> >
>  Hi,
> 
>  I learned to use make-lint!
> 
>  Thanks,
>  F.
> 
> >> Hello,
> >>
> >> NACK, pylint doesn't seem to like the way the fixtures are imported
> >> (pytest does a lot of runtime magic) [1].
> >> One possible solution would be [2]. Though, I don't think this would be
> >> a good idea in our environment. I suggest to create the fixtures on per
> >> module basis.
> >>
> >>
> >> [1]: http://fpaste.org/311949/53118942/
> >> [2]:
> >> https://pytest.org/latest/fixture.html#using-fixtures-from-classes-modules-or-projects
> >>
> >> --
> >> Milan Kubik
> >>
> >>
> > Hi,
> >
> > the fixtures were copied into corresponding module. Please note that this
> > patch has a dependence on my patch 0001 (user plugin).
> >
> > Filip
> Linter:
> * Module ipatests.test_xmlrpc.tracker.group_plugin
> W:100,26: Calling a dict.iter*() method (dict-iter-method)
> 
> please use dict.items
> 
> --
> Milan Kubik
> 
> 

Hi, sorry. This has been fixed in this patch.

Filip
From 08925f2d92b18fdfbc0edc12178eed666bae5d17 Mon Sep 17 00:00:00 2001
From: Filip Skola 
Date: Mon, 9 Nov 2015 16:48:55 +0100
Subject: [PATCH] Refactor test_group_plugin, use GroupTracker for tests

---
 ipatests/test_xmlrpc/test_group_plugin.py | 1758 +
 ipatests/test_xmlrpc/test_stageuser_plugin.py |4 +-
 ipatests/test_xmlrpc/tracker/group_plugin.py  |  146 +-
 3 files changed, 755 insertions(+), 1153 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_group_plugin.py b/ipatests/test_xmlrpc/test_group_plugin.py
index 6eb57c12f18d125de04beefa056f53b4caff1d64..41d28f1cfdbc3d47ea9c47292394637770222ac2 100644
--- a/ipatests/test_xmlrpc/test_group_plugin.py
+++ b/ipatests/test_xmlrpc/test_group_plugin.py
@@ -1,6 +1,7 @@
 # Authors:
 #   Rob Crittenden 
 #   Pavel Zuna 
+#   Filip Skola 

Re: [Freeipa-devel] [PATCH 0005] Refactor test_nesting, create HostGroupTracker

[Filip Skola]

- Original Message -
> On 01/18/2016 02:26 PM, Filip Skola wrote:
> > Hi,
> >
> > this should be fixed in this patch.
> >
> > F.
> >
> > - Original Message -
> >> On 01/15/2016 03:37 PM, Filip Skola wrote:
> >>> Hi,
> >>>
> >>> sending rebased patch.
> >>>
> >>> F.
> >>>
> >>> - Original Message -
>  Hi,
> 
>  the patch no longer applies to master. Please rebase it.
> 
>  Thanks,
>  Milan
> 
>  - Original Message -
>  From: "Filip Skola" 
>  To: freeipa-devel@redhat.com
>  Cc: "Milan Kubík" , "Aleš Mareček"
>  
>  Sent: Tuesday, 22 December, 2015 11:56:15 AM
>  Subject: [PATCH 0005] Refactor test_nesting, create HostGroupTracker
> 
>  Hi,
> 
>  another patch from refactoring-test_xmlrpc series.
> 
>  Filip
> 
> >> NACK, something seems to be missing in the patch
> >>
> >>
> >> * Module ipatests.test_xmlrpc.tracker.hostgroup_plugin
> >> ipatests/test_xmlrpc/tracker/hostgroup_plugin.py:222: [E1101(no-member),
> >> HostGroupTracker.check_add_member_negative] Instance of
> >> 'HostGroupTracker' has no 'adds' member)
> >>
> >> --
> >> Milan Kubik
> >>
> >>
> The same as with patch 0002:
> * Module ipatests.test_xmlrpc.tracker.hostgroup_plugin
> W:142,26: Calling a dict.iter*() method (dict-iter-method)
> 
> Please use dict.items method.
> 
> --
> Milan Kubik
> 
> 

Hi,

attaching a fixed patch. This patch is dependent on updated group plugin test 
patch 0002-7.

Filip
From 5dee3969a039241411ec1648d9ba1b87c149ccf3 Mon Sep 17 00:00:00 2001
From: Filip Skola 
Date: Thu, 28 Jan 2016 10:29:04 +0100
Subject: [PATCH] Refactor test_nesting, create HostGroupTracker

---
 ipatests/test_xmlrpc/test_nesting.py   | 776 -
 ipatests/test_xmlrpc/tracker/group_plugin.py   |   4 +-
 ipatests/test_xmlrpc/tracker/host_plugin.py|   1 +
 .../{group_plugin.py => hostgroup_plugin.py}   | 227 +++---
 4 files changed, 259 insertions(+), 749 deletions(-)
 copy ipatests/test_xmlrpc/tracker/{group_plugin.py => hostgroup_plugin.py} (51%)

diff --git a/ipatests/test_xmlrpc/test_nesting.py b/ipatests/test_xmlrpc/test_nesting.py
index c3bf1ce84e0bef412c44ed847e7e0fc4648a4b74..f78a6e54bd7a94cb9d2645f5bdc5d5c109a79b1f 100644
--- a/ipatests/test_xmlrpc/test_nesting.py
+++ b/ipatests/test_xmlrpc/test_nesting.py
@@ -20,193 +20,93 @@
 Test group nesting and indirect members
 """
 
-from ipalib import api
-from ipatests.test_xmlrpc import objectclasses
-from ipatests.test_xmlrpc.xmlrpc_test import (Declarative, fuzzy_digits,
-  fuzzy_uuid)
-from ipapython.dn import DN
-from ipatests.test_xmlrpc.test_user_plugin import get_user_result
+from ipatests.test_xmlrpc.xmlrpc_test import XMLRPC_test
+from ipatests.test_xmlrpc.tracker.user_plugin import UserTracker
+from ipatests.test_xmlrpc.tracker.group_plugin import GroupTracker
+from ipatests.test_xmlrpc.tracker.host_plugin import HostTracker
+from ipatests.test_xmlrpc.tracker.hostgroup_plugin import HostGroupTracker
 import pytest
 
-group1 = u'testgroup1'
-group2 = u'testgroup2'
-group3 = u'testgroup3'
-group4 = u'testgroup4'
-user1 = u'tuser1'
-user2 = u'tuser2'
-user3 = u'tuser3'
-user4 = u'tuser4'
-
-hostgroup1 = u'testhostgroup1'
-hgdn1 = DN(('cn',hostgroup1),('cn','hostgroups'),('cn','accounts'),
-   api.env.basedn)
-hostgroup2 = u'testhostgroup2'
-hgdn2 = DN(('cn',hostgroup2),('cn','hostgroups'),('cn','accounts'),
-   api.env.basedn)
-
-fqdn1 = u'testhost1.%s' % api.env.domain
-host_dn1 = DN(('fqdn',fqdn1),('cn','computers'),('cn','accounts'),
-  api.env.basedn)
+
+@pytest.fixture(scope='class')
+def user1(request):
+tracker = UserTracker(name=u'tuser1', givenname=u'Test1', sn=u'User1')
+return tracker.make_fixture(request)
+
+
+@pytest.fixture(scope='class')
+def user2(request):
+tracker = UserTracker(name=u'tuser2', givenname=u'Test2', sn=u'User2')
+return tracker.make_fixture(request)
+
+
+@pytest.fixture(scope='class')
+def user3(request):
+tracker = UserTracker(name=u'tuser3', givenname=u'Test3', sn=u'User3')
+return tracker.make_fixture(request)
+
+
+@pytest.fixture(scope='class')
+def user4(request):
+tracker = UserTracker(name=u'tuser4', givenname=u'Test4', sn=u'User4')
+return tracker.make_fixture(request)
+
+
+@pytest.fixture(scope='class')
+def group1(request):
+tracker = GroupTracker(name=u'testgroup1', description=u'Test desc1')
+return tracker.make_fixture(request)
+
+
+@pytest.fixture(scope='class')
+def group2(request):
+tracker = GroupTracker(name=u'testgroup2', description=u'Test desc2')
+return tracker.make_fixture(request)
+
+
+@pytest.fixture(scope='class')
+def group3(request):
+tracker = GroupTracker(name=u'testgroup3', description=u'Test desc3')
+return tracker.make_fixture(request)
+

Re: [Freeipa-devel] [PATCH 0030] Modernize mod_nss's cipher suites

[Martin Basti]

On 22.01.2016 12:32, Martin Kosek wrote:

On 01/21/2016 04:21 PM, Christian Heimes wrote:

The list of supported TLS cipher suites in /etc/httpd/conf.d/nss.conf
has been modernized. Insecure or less secure algorithms such as RC4,
DES and 3DES are removed. Perfect forward secrecy suites with ephemeral
ECDH key exchange have been added. IE 8 on Windows XP is no longer
supported.

The list of enabled cipher suites has been generated with the script
contrib/nssciphersuite/nssciphersuite.py.

The supported suites are currently:

TLS_RSA_WITH_AES_128_CBC_SHA256
TLS_RSA_WITH_AES_256_CBC_SHA256
TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA
TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384
TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA
TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256
TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA
TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384
TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA
TLS_RSA_WITH_AES_128_GCM_SHA256
TLS_RSA_WITH_AES_128_CBC_SHA
TLS_RSA_WITH_AES_256_GCM_SHA384
TLS_RSA_WITH_AES_256_CBC_SHA

https://fedorahosted.org/freeipa/ticket/5589


Thanks for the patch! I updated the ticket to make sure this change is 
release notes.



Hello,

I'm not sure if I'm the right person to do review on this, but I will 
try :-)


1)
Your patch adds whitespace error

Applying: Modernize mod_nss's cipher suites
/home/mbasti/work/freeipa-devel/.git/rebase-apply/patch:52: new blank 
line at EOF.

+
warning: 1 line adds whitespace errors.


2)
+import urllib.request  # pylint: disable=E0611

Please specify pylint disabled check by name

3)
+def update_mod_nss_cipher_suite(http):

in this upgrade, is there any possibility that ciphers might be upgraded 
again in future? (IMO yes).


I think, it can be better to store revision of change instead of boolean

LAST_REVISION =  1

if revision >= LAST_REVISION:
return

sysupgrade.set_upgrade_state('nss.conf', 'cipher_suite_revision', 
LAST_REVISION)



Otherwise it works

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [TEST][Patch 0021] Fixed recent replica installation issues in the lab

[Oleg Fayans]

Guys, could you take a look at this one?

On 01/27/2016 11:16 AM, Oleg Fayans wrote:
> Sorry, trailing whitespace detected. This version passes lint
> 
> On 01/27/2016 09:23 AM, Oleg Fayans wrote:
>> Hi,
>>
>> On 01/21/2016 04:41 PM, Petr Spacek wrote:
>>> Hello,
>>>
>>> On 21.1.2016 13:42, Oleg Fayans wrote:
 freeipa-ofayans-0021-Removed-ip-address-option-from-replica-installation.patch


 From d7ab06a4dcddb919fda351b983d478f1b6968578 Mon Sep 17 00:00:00 2001
 From: Oleg Fayans 
 Date: Thu, 21 Jan 2016 13:30:02 +0100
 Subject: [PATCH] Removed --ip-address option from replica installation

 Explicitly specifying ip-address of the replica messes up with the current
 bind-dyndb-ldap logic, causing reverse zone not to be created.

 Enabled reverse-zone creation for the clients residing in different subnet 
 from
 master
 ---
  ipatests/test_integration/tasks.py | 19 ---
  1 file changed, 12 insertions(+), 7 deletions(-)

 diff --git a/ipatests/test_integration/tasks.py 
 b/ipatests/test_integration/tasks.py
 index 
 6eb55501389c72b4c7aaa599fd4852d7e8f1f3c2..43ef78b0c55deed24a0444f0ac6c38ddb2517481
  100644
 --- a/ipatests/test_integration/tasks.py
 +++ b/ipatests/test_integration/tasks.py
 @@ -69,6 +69,8 @@ def prepare_reverse_zone(host, ip):
  host.run_command(["ipa",
"dnszone-add",
zone], raiseonerr=False)
 +return zone
 +
  
  def prepare_host(host):
  if isinstance(host, Host):
 @@ -319,11 +321,8 @@ def domainlevel(host):
  def replica_prepare(master, replica):
  apply_common_fixes(replica)
  fix_apache_semaphores(replica)
 -prepare_reverse_zone(master, replica.ip)
 -master.run_command(['ipa-replica-prepare',
 -'-p', replica.config.dirman_password,
 -'--ip-address', replica.ip,
 -replica.hostname])
 +master.run_command(['ipa-replica-prepare', '-p', 
 replica.config.dirman_password,
 +'--auto-reverse', replica.hostname])
>>>
>>> I guess that you will need --ip-address option in cases where master's 
>>> reverse
>>> record does not exist (yet).
>> And yo were right. Fixed
>>
>>>
>>> I would recommend you to test this in libvirt or somewhere without revere
>>> records, I suspect that it might blow up.
>>>
  replica_bundle = master.get_file_contents(
  paths.REPLICA_INFO_GPG_TEMPLATE % replica.hostname)
  replica_filename = get_replica_filename(replica)
 @@ -339,8 +338,7 @@ def install_replica(master, replica, setup_ca=True, 
 setup_dns=False,
  # and replica installation would fail
  args = ['ipa-replica-install', '-U',
  '-p', replica.config.dirman_password,
 -'-w', replica.config.admin_password,
 -'--ip-address', replica.ip]
 +'-w', replica.config.admin_password]
  if setup_ca:
  args.append('--setup-ca')
  if setup_dns:
 @@ -380,6 +378,13 @@ def install_client(master, client, extra_args=()):
  client.collect_log(paths.IPACLIENT_INSTALL_LOG)
  
  apply_common_fixes(client)
 +# Now, for the situations where a client resides in a different 
 subnet from
 +# master, we need to explicitly tell master to create a reverse zone 
 for
 +# the client and enable dynamic updates for this zone.
 +allow_sync_ptr(master)
 +zone = prepare_reverse_zone(master, client.ip)
 +master.run_command(["ipa", "dnszone-mod", zone,
 +"--dynamic-update=TRUE"], raiseonerr=False)
>>>
>>> I'm not a big fan of ignoring exceptions here, it might be better to
>>> encapsulate the first command with try: except: and run the zone-mod only if
>>> the add worked as expected.
>>>
>>> Also, logging an message that reverse zone was not added might be a good 
>>> idea.
>> Agreed. Done.
>>
>>>
>>> HTH
>>>
>>> Petr^2 Spacek
>>>
>>>
  
  client.run_command(['ipa-client-install', '-U',
  '--domain', client.domain.name,
>>>
>>
>>
>>
> 
> 
> 

-- 
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 154] ipa-kdb: map_groups() consider all results

[Petr Vobornik]

On 01/05/2016 07:55 PM, Sumit Bose wrote:

Hi,

to find out to which local group a external user is mapped we do a
dereference search over the external groups with the SIDs related to the
external user. If a SID is mapped to more than one external group we
currently consider only the first returned match. With this patch all
results are taken into account. This makes sure all expected local group
memberships are added to the PAC which resolves
https://fedorahosted.org/freeipa/ticket/5573.

bye,
Sumit



bump for review.
--
Petr Vobornik

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [PATCH] 0008 Refactor test_sudocmdgroup_plugin, create SudoCmdGroupTracker

[Filip Skola]

Hi,

sending the next sudo patch. This one depends on the previous one 
(sudocmd_plugin).

Filip
From 9aab785cf39233998f42917b10cab3ef976f91f1 Mon Sep 17 00:00:00 2001
From: Filip Skola 
Date: Thu, 28 Jan 2016 11:57:08 +0100
Subject: [PATCH] Refactor test_sudocmdgroup_plugin, create SudoCmdGroupTracker

---
 ipatests/test_xmlrpc/test_sudocmdgroup_plugin.py   | 854 +
 .../test_xmlrpc/tracker/sudocmdgroup_plugin.py | 234 ++
 2 files changed, 433 insertions(+), 655 deletions(-)
 create mode 100644 ipatests/test_xmlrpc/tracker/sudocmdgroup_plugin.py

diff --git a/ipatests/test_xmlrpc/test_sudocmdgroup_plugin.py b/ipatests/test_xmlrpc/test_sudocmdgroup_plugin.py
index c72ba2f7a25d5b3c4817c7e17346a8ff5a324760..74575e2c846dcdc6db29f1bab454543da8e5d856 100644
--- a/ipatests/test_xmlrpc/test_sudocmdgroup_plugin.py
+++ b/ipatests/test_xmlrpc/test_sudocmdgroup_plugin.py
@@ -21,677 +21,221 @@ Test the `ipalib/plugins/sudocmdgroup.py` module.
 """
 
 from ipalib import api, errors
-from ipatests.test_xmlrpc import objectclasses
-from ipatests.test_xmlrpc.xmlrpc_test import (Declarative, fuzzy_uuid,
-  fuzzy_sudocmddn)
-from ipapython.dn import DN
+from ipatests.util import assert_deepequal
+from ipatests.test_xmlrpc.xmlrpc_test import (XMLRPC_test, fuzzy_sudocmddn,
+  raises_exact)
+from ipatests.test_xmlrpc.tracker.sudocmdgroup_plugin import SudoCmdGroupTracker
+from ipatests.test_xmlrpc.test_sudocmd_plugin import create_sudocmd, delete_sudocmd
 import pytest
 
-sudocmdgroup1 = u'testsudocmdgroup1'
-sudocmdgroup2 = u'testsudocmdgroup2'
-sudocmd1 = u'/usr/bin/sudotestcmd1'
-sudocmd1_camelcase = u'/usr/bin/sudoTestCmd1'
-sudocmd_plus = u'/bin/ls -l /lost+found/*'
-
-def create_command(sudocmd):
-return dict(
-desc='Create %r' % sudocmd,
-command=(
-'sudocmd_add', [], dict(sudocmd=sudocmd,
-description=u'Test sudo command')
-),
-expected=dict(
-value=sudocmd,
-summary=u'Added Sudo Command "%s"' % sudocmd,
-result=dict(
-objectclass=objectclasses.sudocmd,
-sudocmd=[sudocmd],
-ipauniqueid=[fuzzy_uuid], description=[u'Test sudo command'],
-dn=fuzzy_sudocmddn,
-),
-),
-)
 
+sudocmd1_desc = u'Test sudo command 1'
+sudocmd2_desc = u'Test sudo command 2'
+sudocmd_plus_desc = u'Test sudo command 3'
 
-@pytest.mark.tier1
-class test_sudocmdgroup(Declarative):
-cleanup_commands = [
-('sudocmdgroup_del', [sudocmdgroup1], {}),
-('sudocmdgroup_del', [sudocmdgroup2], {}),
-('sudocmd_del', [sudocmd1], {}),
-('sudocmd_del', [sudocmd1_camelcase], {}),
-('sudocmd_del', [sudocmd_plus], {}),
-]
-
-tests = [
-
-
-# create sudo command
-dict(
-desc='Create %r' % sudocmd1,
-command=(
-'sudocmd_add', [], dict(sudocmd=sudocmd1, description=u'Test sudo command 1')
-),
-expected=dict(
-value=sudocmd1,
-summary=u'Added Sudo Command "%s"' % sudocmd1,
-result=dict(
-objectclass=objectclasses.sudocmd,
-sudocmd=[u'/usr/bin/sudotestcmd1'],
-ipauniqueid=[fuzzy_uuid],
-description=[u'Test sudo command 1'],
-dn=fuzzy_sudocmddn,
-),
-),
-),
-
-dict(
-desc='Create %r' % sudocmd1_camelcase,
-command=(
-'sudocmd_add', [], dict(sudocmd=sudocmd1_camelcase, description=u'Test sudo command 2')
-),
-expected=dict(
-value=sudocmd1_camelcase,
-summary=u'Added Sudo Command "%s"' % sudocmd1_camelcase,
-result=dict(
-objectclass=objectclasses.sudocmd,
-sudocmd=[u'/usr/bin/sudoTestCmd1'],
-ipauniqueid=[fuzzy_uuid],
-description=[u'Test sudo command 2'],
-dn=fuzzy_sudocmddn,
-),
-),
-),
 
-dict(
-desc='Verify the managed sudo command %r was created' % sudocmd1,
-command=('sudocmd_show', [sudocmd1], {}),
-expected=dict(
-value=sudocmd1,
-summary=None,
-result=dict(
-sudocmd=[sudocmd1],
-description=[u'Test sudo command 1'],
-dn=fuzzy_sudocmddn,
-),
-),
-),
-
-
-
-# create sudo command group1:
-dict(
-desc='Try to retrieve non-existent %r' % sudocmdgroup1,
-command=('sudocmdgroup_show', [sudocmdgroup1], {}),
-

[Freeipa-devel] [PATCH] 0003 webui: Issue New Certificate dialogs validates data

[Pavel Vomacka]

Hello,

I made a patch for the https://fedorahosted.org/freeipa/ticket/5432 
ticket. All Issue new certificate dialogs now validates input data.


--
Pavel Vomacka
>From 2d323f8cf997af594de01405b9d242360f625b86 Mon Sep 17 00:00:00 2001
From: Pavel Vomacka 
Date: Thu, 28 Jan 2016 12:07:26 +0100
Subject: [PATCH] Add validation to Issue new certificate dialog

'Issue new certificate' dialog now validates whether user fills 'principal' and 'csr' field.
In case that one of these fields is empty then it does not allow to submit the dialog.

https://fedorahosted.org/freeipa/ticket/5432
---
 install/ui/src/freeipa/certificate.js | 95 +--
 install/ui/src/freeipa/details.js |  5 ++
 2 files changed, 84 insertions(+), 16 deletions(-)

diff --git a/install/ui/src/freeipa/certificate.js b/install/ui/src/freeipa/certificate.js
index 93f3cfc68a95bfb8014aaf96d1b571568ac605dc..dbdad98e07e239339cab28548d30493c996f8da6 100755
--- a/install/ui/src/freeipa/certificate.js
+++ b/install/ui/src/freeipa/certificate.js
@@ -30,10 +30,11 @@ define([
 './reg',
 './rpc',
 './text',
+'./widget',
 './dialog'],
 function(
 lang, builder, metadata_provider, IPA, $, menu,
-phases, reg, rpc, text) {
+phases, reg, rpc, text, widget_mod) {
 
 var exp = IPA.cert = {};
 
@@ -399,11 +400,49 @@ IPA.cert.request_dialog = function(spec) {
 spec = spec || {};
 
 spec.sections = spec.sections || [];
-var section = { fields: [] };
-spec.sections.push(section);
+var section0 = { fields: [] };
+var section_message = {
+show_header: false,
+fields: [
+{
+field: false, // the section does not contain any field.
+$type: 'html',
+name: 'message_html_widget',
+html: spec.message
+}
+],
+layout:
+{
+$factory: widget_mod.fluid_layout,
+widget_cls : "col-sm-12 controls",
+label_cls : "hide"
+}
+};
+
+var section_csr = {
+show_header: false,
+fields:
+[
+{
+$type: 'textarea',
+name: 'textarea_cert',
+required: true
+}
+],
+layout:
+{
+$factory: widget_mod.fluid_layout,
+widget_cls : "col-sm-12 controls",
+label_cls : "hide"
+}
+};
+
+spec.sections.push(section0);
+spec.sections.push(section_message);
+spec.sections.push(section_csr);
 
 if (spec.show_principal) {
-section.fields.push(
+section0.fields.push(
 {
 $type: 'text',
 name: 'principal',
@@ -418,7 +457,7 @@ IPA.cert.request_dialog = function(spec) {
 }
 );
 }
-section.fields.push(
+section0.fields.push(
 {
 $type: 'entity_select',
 name: 'profile_id',
@@ -443,7 +482,26 @@ IPA.cert.request_dialog = function(spec) {
 click: function() {
 var values = {};
 that.save(values);
-var request = $.trim(that.textarea.val());
+var csr_textarea_format = that.get_field('textarea_cert');
+
+// check requested fields
+if (!that.validate()) {
+var dialog_fields = that.fields.get_fields();
+
+for (var i=0; i

Re: [Freeipa-devel] [PATCH 0131] fix standalone installation of externally signed CA on IPA master

[Martin Basti]

On 26.01.2016 13:44, Martin Babinsky wrote:

On 01/26/2016 01:06 PM, Martin Babinsky wrote:

https://fedorahosted.org/freeipa/ticket/5636



This also happens on ipa-4-3/master but the offending check was moved 
around. Attaching patch for 4-3/master branches.





ACK for master and ipa-4-3

I continue with testing on ipa-4-2 to be sure
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0128] ipalib/cli.py: pythonify Collector class

[Tomas Babej]

On 01/27/2016 03:58 PM, Martin Babinsky wrote:
> On 01/18/2016 06:43 PM, Martin Babinsky wrote:
>> A little patch that should make some future pylint errors disappear.
>>
>>
>>
> Attaching updated patch that does not promote direct molestation of
> instance dictionaries.
> 
> 
> 

Patch looks good, one thing I am concerened about though is that
__todict__ now returns a direct reference to the internal, mutable dict,
and no longer a (shallow) copy.

Maybe we should use dict.copy() there?

Tomas

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0131] fix standalone installation of externally signed CA on IPA master

[Martin Basti]

On 28.01.2016 14:49, Martin Basti wrote:



On 26.01.2016 13:44, Martin Babinsky wrote:

On 01/26/2016 01:06 PM, Martin Babinsky wrote:

https://fedorahosted.org/freeipa/ticket/5636



This also happens on ipa-4-3/master but the offending check was moved 
around. Attaching patch for 4-3/master branches.





ACK for master and ipa-4-3

I continue with testing on ipa-4-2 to be sure



ACK

master: 72e72615df8b178ebbcb2e4944ba289ef263c951
ipa-4-3: 87cd18892fcbc520c8d45c5f7624a909c9347779
ipa-4-2: 24384624b3ad2eb0e5ffe6483c34156c7d335888
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code