Re: [Freeipa-devel] design review: Certificate Profiles

2015-04-17 Thread Simo Sorce
On Fri, 2015-04-17 at 14:08 +0200, Martin Kosek wrote:
> On 04/16/2015 10:03 AM, Fraser Tweedale wrote:
> > Hi everyone,
> >
> > Please review my Certificate Profiles design proposal:
> > http://www.freeipa.org/page/V4/Certificate_Profiles
> >
> > Let me know what is unclear, what needs expansion, and what is plain
> > wrong :)
> >
> > The schema for storing multiple certificates for a principal is
> > still being discussed but I expect it will be agreed soon, and I
> > will add it to the document.
> >
> > I am revising the sub-CAs design proposal and it will soon be
> > published for review as well.
> 
> 1) here did you get this feature template? It is the one that is obsolete 
> (header levels, document structure, missing author in the box)... This is the 
> right template:
> http://www.freeipa.org/page/Feature_template
> 
> 2) I miss certprofile-find command - to enable Web UI and/or CLI to search 
> through existing profiles.
> 
> 3) Permissions
> So your plan is to allow different groups use different profiles? So there 
> would be for example profiles allowed to all users (something like 
> userCattegory:all that we use for HBAC/SUDO)? How do you plan to deal with 
> authorization? Will be on a FreeIPA framework level or for example by DS ACIs 
> that would simply not show the profiles?

Keep in mind our design philosophy from the start was: the framework
only have the privileges of the user accessing it and makes no ACI
decisions.

We broke that abstraction with the RA agent stuff, but I plan on fixing
it some days by taking it away from the framework again, so I would not
be favorable to see more Access control implemented in the framework
unless there is no other sane way.

Simo.

> 4) Searching for certificates by profile - FEEDBACK REQUIRED
> It would be nice to incorporate this filter to current cert-find command.
> 
> 5) Default set of profiles
> Should we also propose a basic set of canned profiles so that I can picture 
> what will be the possibilities?
> 
> Would it be something like
> * Server profile
> * Client profile
> 
> 6) Upgrades
> It may happen that FreeIPA needs to upgrade defaults of a canned profile. It 
> would be nice to have a section how it would do it.
> 
> This is all I could think of so far.
> 


-- 
Simo Sorce * Red Hat, Inc * New York

-- 
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] 821 webui: add pwpolicy link to group details page if group has associated pwpolicy

2015-04-17 Thread Petr Vobornik

https://fedorahosted.org/freeipa/ticket/4982
--
Petr Vobornik
From c57d0309b573d97e13e080e9be0f1db7da83c1da Mon Sep 17 00:00:00 2001
From: Petr Vobornik 
Date: Fri, 17 Apr 2015 17:56:58 +0200
Subject: [PATCH] webui: add pwpolicy link to group details page if group has
 associated pwpolicy

https://fedorahosted.org/freeipa/ticket/4982
---
 install/ui/src/freeipa/group.js  | 10 +-
 install/ui/src/freeipa/widget.js | 25 +++--
 2 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/install/ui/src/freeipa/group.js b/install/ui/src/freeipa/group.js
index 3961384dc4b2e769ddaa0b2d3f165cc48ad471a1..6f6f13e74737de9c79daa9d7d0ad156365a08214 100644
--- a/install/ui/src/freeipa/group.js
+++ b/install/ui/src/freeipa/group.js
@@ -68,7 +68,15 @@ return {
 posixgroup: '@i18n:objects.group.posix'
 }
 },
-'gidnumber'
+'gidnumber',
+{
+$type: 'link',
+name: 'pwpolicy',
+param: 'cn',
+label: '@mo:pwpolicy.label_singular',
+other_entity: 'pwpolicy',
+require_link: true
+}
 ]
 }
 ],
diff --git a/install/ui/src/freeipa/widget.js b/install/ui/src/freeipa/widget.js
index 29d320c07fe156a807ad2bf4415b3058a8a71e98..125d499b683557fec9f9ece6007d3560f03ee025 100644
--- a/install/ui/src/freeipa/widget.js
+++ b/install/ui/src/freeipa/widget.js
@@ -4143,6 +4143,12 @@ IPA.link_widget = function(spec) {
  */
 that.no_check = spec.no_check;
 
+/**
+ * Whether value can be displayed even if link is not valid.
+ * @property {boolean}
+ */
+that.require_link = spec.require_link !== undefined ? spec.require_link : false;
+
 that.value = '';
 that.values = [];
 
@@ -4182,18 +4188,17 @@ IPA.link_widget = function(spec) {
 };
 
 that.update_link = function() {
+
+var link = false;
+var nonlink = false;
+
 if (that.value) {
-if(that.is_link) {
-that.link.css('display','');
-that.nonlink.css('display','none');
-} else {
-that.link.css('display','none');
-that.nonlink.css('display','');
-}
-} else {
-that.link.css('display','none');
-that.nonlink.css('display','none');
+link = !!that.is_link;
+nonlink = !that.is_link && !that.require_link;
 }
+
+that.link.css('display', link ? '' : 'none');
+that.nonlink.css('display', nonlink ? '' : 'none');
 };
 
 /**
-- 
2.1.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] User life cycle: How to update 60basev3.ldif

2015-04-17 Thread thierry bordaz

Hello,

   User life cycle uses a new DS aci right: moddn. This right comes
   with two new target keywords (target_to and target_from).
   permission plugins should support those new target keywords and so
   those attributes need to be defined in the schema 60basev3.ldif.

   When adding new attributes in that schema, I should pick new OIDs.
   Is it ok to pick the next ones available in ds-oids/08-FreeIPA.txt
   (rhanana) and what is review process for changes in 08-FreeIPA.txt ?

   thanks
   thierry

-- 
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] 819-820 jQuery.ordered_map: faster creation

2015-04-17 Thread Petr Vobornik
Improve performance of Web UI with very large user groups or any other 
usage which adds a lot of values into jQuery.ordered_map.

--
Petr Vobornik
From f11682039f41ce27f770637e67817b9ccdfad0af Mon Sep 17 00:00:00 2001
From: Petr Vobornik 
Date: Fri, 17 Apr 2015 16:12:21 +0200
Subject: [PATCH] jQuery.ordered_map: remove map attribute

map attribute is redundant and not used.

Use `get` method instead.
---
 install/ui/src/libs/jquery.ordered-map.js | 10 ++
 install/ui/test/ordered_map_tests.js  |  5 -
 2 files changed, 2 insertions(+), 13 deletions(-)

diff --git a/install/ui/src/libs/jquery.ordered-map.js b/install/ui/src/libs/jquery.ordered-map.js
index 19f7b3dd5e2378fa38206fadda61fe4f06b71fd3..b25dc3379fc1c77e526bcdb9d177fb7e54ec7c48 100644
--- a/install/ui/src/libs/jquery.ordered-map.js
+++ b/install/ui/src/libs/jquery.ordered-map.js
@@ -30,11 +30,10 @@ jQuery.ordered_map = jQuery.fn.ordered_map = function(map) {
  */
 that.keys = [];
 that.values = [];
-that.map = {};
 that.length = 0;
 
 that.get = function(key) {
-return that.map[key];
+return that.values[that._key_indicies[key]];
 };
 
 that.put = function(key, value, position) {
@@ -60,8 +59,6 @@ jQuery.ordered_map = jQuery.fn.ordered_map = function(map) {
 }
 }
 
-that.map[key] = value;
-
 return that;
 };
 
@@ -111,11 +108,9 @@ jQuery.ordered_map = jQuery.fn.ordered_map = function(map) {
 var i = that.get_key_index(key);
 if (i<0) return null;
 
+var value = that.values[i];
 that.keys.splice(i, 1);
 that.values.splice(i, 1);
-
-var value = that.map[key];
-delete that.map[key];
 delete that._key_indicies[key];
 that.length = that.keys.length;
 return value;
@@ -124,7 +119,6 @@ jQuery.ordered_map = jQuery.fn.ordered_map = function(map) {
 that.empty = function() {
 that.keys = [];
 that.values = [];
-that.map = {};
 that._key_indicies = {};
 that.length = that.keys.length;
 return that;
diff --git a/install/ui/test/ordered_map_tests.js b/install/ui/test/ordered_map_tests.js
index f3d2d5b36aebea5ff3f4e7b6ddadb30a155b1156..f302de5d90de82e5967782de8e9f34af43d34a4f 100755
--- a/install/ui/test/ordered_map_tests.js
+++ b/install/ui/test/ordered_map_tests.js
@@ -30,7 +30,6 @@ test("Testing $.ordered_map constructor.", function() {
 strictEqual(test.length, 0, "Checking length.");
 deepEqual(test.keys, [], "Checking keys.");
 deepEqual(test.values, [], "Checking values.");
-deepEqual(test.map, {}, "Checking map.");
 });
 
 test("Testing $.ordered_map.put().", function() {
@@ -84,7 +83,6 @@ test("Testing $.ordered_map.put().", function() {
 strictEqual(test.length, 8, 'Checking length.');
 deepEqual(test.keys, [key5, key4, key1, key3, key2, key6, key7, key8], 'Checking keys.');
 deepEqual(test.values, [value5, value4, value1, value3, value2, value6, value7, value8], 'Checking values.');
-deepEqual(test.map, map, 'Checking map.');
 });
 
 test("Testing $.ordered_map.get().", function() {
@@ -110,7 +108,6 @@ test("Testing $.ordered_map.get().", function() {
 strictEqual(test.length, 2, 'Checking length.');
 deepEqual(test.keys, [key1, key2], 'Checking keys.');
 deepEqual(test.values, [value1, value2], 'Checking values.');
-deepEqual(test.map, map, 'Checking map.');
 strictEqual(result1, value1, 'Checking result 1.');
 strictEqual(result2, value2, 'Checking result 2.');
 });
@@ -136,7 +133,6 @@ test("Testing $.ordered_map.remove().", function() {
 strictEqual(test.length, 1, 'Checking length.');
 deepEqual(test.keys, [key2], 'Checking keys.');
 deepEqual(test.values, [value2], 'Checking values.');
-deepEqual(test.map, map, 'Checking map.');
 strictEqual(result1, value1, 'Checking result.');
 });
 
@@ -158,7 +154,6 @@ test("Testing $.ordered_map.empty().", function() {
 strictEqual(test.length, 0, 'Checking length.');
 deepEqual(test.keys, [], 'Checking keys.');
 deepEqual(test.values, [], 'Checking values.');
-deepEqual(test.map, {}, 'Checking map.');
 });
 
 };});
\ No newline at end of file
-- 
2.1.0

From 001522023eef1c2f08c822297c8c0629f15c8e7d Mon Sep 17 00:00:00 2001
From: Petr Vobornik 
Date: Thu, 16 Apr 2015 18:54:25 +0200
Subject: [PATCH] jQuery.ordered_map: faster creation

Creation of map with e.g. 30K values was very slow. Map checked if a value is
in in the map but it used Array's indexOf method therefore the complexity was
quadratic instead of linear.
---
 install/ui/src/libs/jquery.ordered-map.js | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/install/ui/src/libs/jquery.ordered-map.js b/install/ui/src/libs/jquery.ordered-map.js
index 77d17c56ac926f1193c97e0cc46ceebe002005c6..19f7b3dd5e2378fa38206fadda61fe4f06b71fd3 100644
--- a/install/ui/src/libs/jquery.ordered-map.js
+++ b/install/ui/src/libs/j

Re: [Freeipa-devel] [PATCH 424] install: Introduce installer framework ipapython.install

2015-04-17 Thread Jan Cholasta

Dne 16.4.2015 v 16:46 Jan Cholasta napsal(a):

Hi,

the attached patch adds the basics of the new installer framework.

As a next step, I plan to convert the install scripts to use the
framework with their old code (the old code will be gradually ported to
the framework later).

(Note I didn't manage to write docstrings today, expect update tomorrow.)


Added some docstrings.

Also updated the patch to reflect little brainstorming David and I had 
this morning.




Honza


--
Jan Cholasta
>From 2245f992185a6f03b27afa50b10c811797c6dda4 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Thu, 16 Apr 2015 14:35:24 +
Subject: [PATCH] install: Introduce installer framework ipapython.install

---
 ipapython/install.py | 468 +++
 1 file changed, 468 insertions(+)
 create mode 100644 ipapython/install.py

diff --git a/ipapython/install.py b/ipapython/install.py
new file mode 100644
index 000..1c0c646
--- /dev/null
+++ b/ipapython/install.py
@@ -0,0 +1,468 @@
+# Authors:
+#   Jan Cholasta 
+#
+# Copyright (C) 2015  Red Hat
+# see file 'COPYING' for use and warranty information
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+"""
+Installer framework.
+"""
+
+import sys
+import inspect
+import abc
+import itertools
+
+_VALIDATE_PENDING = 'VALIDATE_PENDING'
+_VALIDATE_RUNNING = 'VALIDATE_RUNNING'
+_EXECUTE_PENDING = 'EXECUTE_PENDING'
+_EXECUTE_RUNNING = 'EXECUTE_RUNNING'
+_STOPPED = 'STOPPED'
+_FAILED = 'FAILED'
+_CLOSED = 'CLOSED'
+
+_missing = object()
+
+
+class from_(object):
+__slots__ = ('obj',)
+
+def __init__(self, obj):
+self.obj = obj
+
+
+def _run_generator_with_yield_from(gen):
+exc_info = None
+value = None
+
+stack = [gen]
+while stack:
+prev_exc_info, exc_info = exc_info, None
+prev_value, value = value, None
+
+gen = stack[-1]
+try:
+if prev_exc_info is None:
+value = gen.send(prev_value)
+else:
+value = gen.throw(*prev_exc_info)
+except StopIteration:
+stack.pop()
+continue
+except BaseException:
+exc_info = sys.exc_info()
+stack.pop()
+continue
+else:
+if isinstance(value, from_):
+value = None
+stack.append(value.obj)
+continue
+
+try:
+value = (yield value)
+except BaseException:
+exc_info = sys.exc_info()
+
+if exc_info:
+raise exc_info  #pylint: disable=raising-bad-type
+
+
+class InvalidStateError(Exception):
+pass
+
+
+class Knob(object):
+"""
+Public argument of a Configurator.
+"""
+
+__counter = itertools.count()
+
+def __init__(self, type, default=_missing, label=None):
+"""
+Initialize the knob.
+"""
+self.type = type
+if default is _missing:
+self.required = True
+self.default = None
+else:
+self.required = False
+self.default = default
+self.label = label
+self.order = next(self.__counter)
+
+def _set(self, obj, value):
+setattr(obj, '_knob{0}'.format(id(self)), value)
+
+def __get__(self, obj, obj_type):
+if obj is None:
+return self
+return getattr(obj, '_knob{0}'.format(id(self)))
+
+
+class Configurator(object):
+"""
+Base class of all configurators.
+
+FIXME: details of validate/execute, args and knobs
+"""
+
+__metaclass__ = abc.ABCMeta
+
+@classmethod
+def args(cls):
+"""
+Iterate over arguments of the configurator.
+"""
+for super_cls in cls.__mro__:
+if not issubclass(super_cls, Configurator):
+continue
+
+try:
+init = super_cls.__dict__['__init__']
+except KeyError:
+continue
+
+argspec = inspect.getargspec(init)
+for name in argspec.args[1:]:
+if not name.startswith('_'):
+yield name
+
+for knob_cls, name, obj in cls.knobs():
+yield name
+
+@classmethod
+def knobs(cls):
+"""
+Iterate over knobs defined for the configurator.
+"""
+result = []
+for name in dir(cls):
+

Re: [Freeipa-devel] [PATCH 0014] emit a more helpful error messages when CA configuration fails

2015-04-17 Thread Martin Babinsky

On 03/05/2015 01:11 PM, Martin Babinsky wrote:

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



___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel



Nobody to review this?

--
Martin^3 Babinsky

--
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] design review: Certificate Profiles

2015-04-17 Thread Milan Kubik

On 04/16/2015 10:03 AM, Fraser Tweedale wrote:

Hi everyone,

Please review my Certificate Profiles design proposal:
http://www.freeipa.org/page/V4/Certificate_Profiles

Let me know what is unclear, what needs expansion, and what is plain
wrong :)

The schema for storing multiple certificates for a principal is
still being discussed but I expect it will be agreed soon, and I
will add it to the document.

I am revising the sub-CAs design proposal and it will soon be
published for review as well.

Cheers,
Fraser


Hello Fraser,

I will reiterate one of my concernes from our private mails here for the 
wider audience :)


I'd really like to have a way how to list the profiles managed by IPA 
other than using
the dogtag REST API directly. Simple wrapper around the api call for 
/ca/rest/profiles[/$id[/raw]]
endpoints returning a list of IDs [and dumping the profile to file] for 
the sake of consistency,
since other endpoints are wrapped by ipa commands, would be sufficient 
for me.


This can be also used to query the information (at least the list of 
IDs) when used in the web UI.


I don't know how exactly dogtag is wired into IPA (I've seen that there 
is separate suffix
on the DS instance) and I don't really need to duplicate any data into 
the defaultNamingContext

and its subtree.


Cheers,
Milan

-- 
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 0029] suppress errors arising from deleting non-existent files during client uninstall

2015-04-17 Thread Martin Babinsky

On 04/17/2015 12:41 PM, Martin Babinsky wrote:

On 04/17/2015 12:36 PM, Martin Basti wrote:

On 17/04/15 12:33, Martin Babinsky wrote:

On 04/17/2015 12:04 PM, Martin Basti wrote:

On 15/04/15 15:53, Martin Babinsky wrote:

On 04/14/2015 04:24 PM, Martin Basti wrote:

On 14/04/15 16:12, Martin Basti wrote:

On 14/04/15 14:25, Martin Babinsky wrote:

This patch addresses https://fedorahosted.org/freeipa/ticket/4966

The noise during rollback/uninstall is caused mainly by
unsuccessful
attempts to remove files that do not exist anymore. These errors
are
now logged at debug level and do not pop-up to stdout/stderr.




Hello, thank you for the patch.

1)
The option add_warning is quite unclear to me. It does not show
warning but error. I suggest something like,  show_hint,
show_user_action, or something show_additional_..., or
promt_manual_removal

Martin^2



Continue...

2)

 if file_exists(preferences_fname):
 try:
 os.remove(preferences_fname)
 except OSError as e:
 log_file_removal_error(e, preferences_fname,
True)

In this case file not found error should never happen.

Could you remove the 'if file_exists' part and handle just exception?


I just reverted this bit to original form in order to not fix
something that isn't broken. Is that ok?

3)
this is inconsistent with change above, choose one style please:

 if os.path.exists(ca_file):
 try:
 os.unlink(ca_file)
 except OSError, e:
 root_logger.error(
 "Failed to remove '%s': %s", ca_file, e)

--
Martin Basti



Attaching updated patch.


thanks,

just one nitpick, can you move the new function into installutils, it
can be used in different scripts not just in ipaclient.



I'm not sure if it is a good idea as installutils is a part for
freeipa-server package.

Placing it there would create an unnecessary dependency of
freeipa-client on freeipa-server because of a single function.


you are right, I do not why I thought that ipa-client-install uses
installutils.

ACK


self-NACK, I will try to rewrite the patch in a slightly less dumb way.

Sorry for the confusion.



Attaching updated patch which does the same but using a wrapper around 
os.remove().


Jan suggested to keep the new function in 'ipa-client-install' and move 
it around when we do installer re#$%@^ing.


Is that ok?

--
Martin^3 Babinsky
From 2d8ec0fbccad864850495acea423558b99f8e1f4 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Tue, 14 Apr 2015 13:55:33 +0200
Subject: [PATCH] suppress errors arising from deleting non-existent files
 during client uninstall

When rolling back partially configured IPA client a number of OSErrors pop up
due to uninstaller trying to remove files that do not exist anymore. This
patch supresses these errors while keeping them in log as debug messages.

https://fedorahosted.org/freeipa/ticket/4966
---
 ipa-client/ipa-install/ipa-client-install | 46 +++
 1 file changed, 28 insertions(+), 18 deletions(-)

diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 3f9a7419a10ddcb4618e80789a06a05058d1e8a4..b38d71e956faf02b88ab20d3aa8e42be4b45d78d 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -224,6 +224,31 @@ def logging_setup(options):
 console_format='%(message)s')
 
 
+def remove_file(filename, prompt_manual_removal=False):
+"""
+Deletes a file. If the file does not exist (OSError 2),
+logs a debug message. Otherwise logs an error.
+:param filename: name of the file to be removed
+:param prompt_manual_removal: if set to `True` prints an additional
+message instructing the user to delete the file manually.
+Default is `False`.
+"""
+logger_func = root_logger.error
+
+try:
+os.remove(filename)
+except OSError as e:
+if e.errno == 2:
+logger_func = root_logger.debug
+prompt_manual_removal = False
+
+logger_func("Failed to remove file %s: %s", filename, e)
+
+if prompt_manual_removal:
+logger_func('Please remove %s manually, as it can cause '
+'subsequent installation to fail.', filename)
+
+
 def log_service_error(name, action, error):
 root_logger.error("%s failed to %s: %s", name, action, str(error))
 
@@ -529,10 +554,7 @@ def uninstall(options, env):
  os.path.join(ipa_db.secdir, 'key3.db'),
  os.path.join(ipa_db.secdir, 'secmod.db'),
  os.path.join(ipa_db.secdir, 'pwdfile.txt')):
-try:
-os.remove(filename)
-except OSError, e:
-root_logger.error("Failed to remove %s: %s", filename, e)
+remove_file(filename)
 
 for nickname, trust_flags in ipa_certs:
 while sys_db.has_nic

Re: [Freeipa-devel] design review: Certificate Profiles

2015-04-17 Thread Martin Kosek

On 04/16/2015 10:03 AM, Fraser Tweedale wrote:

Hi everyone,

Please review my Certificate Profiles design proposal:
http://www.freeipa.org/page/V4/Certificate_Profiles

Let me know what is unclear, what needs expansion, and what is plain
wrong :)

The schema for storing multiple certificates for a principal is
still being discussed but I expect it will be agreed soon, and I
will add it to the document.

I am revising the sub-CAs design proposal and it will soon be
published for review as well.


1) here did you get this feature template? It is the one that is obsolete 
(header levels, document structure, missing author in the box)... This is the 
right template:

http://www.freeipa.org/page/Feature_template

2) I miss certprofile-find command - to enable Web UI and/or CLI to search 
through existing profiles.


3) Permissions
So your plan is to allow different groups use different profiles? So there 
would be for example profiles allowed to all users (something like 
userCattegory:all that we use for HBAC/SUDO)? How do you plan to deal with 
authorization? Will be on a FreeIPA framework level or for example by DS ACIs 
that would simply not show the profiles?


4) Searching for certificates by profile - FEEDBACK REQUIRED
It would be nice to incorporate this filter to current cert-find command.

5) Default set of profiles
Should we also propose a basic set of canned profiles so that I can picture 
what will be the possibilities?


Would it be something like
* Server profile
* Client profile

6) Upgrades
It may happen that FreeIPA needs to upgrade defaults of a canned profile. It 
would be nice to have a section how it would do it.


This is all I could think of so far.

--
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] otptoken_yubikey, append CR by default and add a option for not doing so

2015-04-17 Thread Nathaniel McCallum
On Thu, 2015-04-16 at 09:12 +0200, Jan Cholasta wrote:
> Dne 9.4.2015 v 15:11 Luc de Louw napsal(a):
> > 
> > On 04/09/2015 02:28 PM, Jan Cholasta wrote:
> > > > > > Let's say you now introduce --no-cr flag. What if we 
> > > > > > decide to change
> > > > > > the default to False? How would you then change the 
> > > > > > option/API?
> > > > > 
> > > > > You would have to add --cr flag.
> > > > 
> > > > That was the point - some clients would send "ct" flag, some 
> > > > "no_cr"
> > > > and there
> > > > would have to be special handling.
> > > > 
> > > > > > It is more flexible IMO to just use something like
> > > > > > 
> > > > > > --cr=TRUE|FALSE with TRUE being the default
> > > > > 
> > > > > I would say --append-cr=TRUE|FALSE with no default, meaning 
> > > > > do not
> > > > > add the flag
> > > > > to the config at all.
> > > > 
> > > > I though the idea was to append the CR by default, i.e.
> > > > --append-cr=TRUE|FALSE
> > > > with TRUE being the default.
> > > > 
> > > 
> > > If you want to hardcode the default into the plugin, there is no 
> > > benefit
> > > in using Bool over Flag, because Flag is actually a Bool with 
> > > hardcoded
> > > default value.
> > > 
> > 
> > I actually started with a bool, default=True. I had the problem 
> > that the
> > Default value was ignored, the value was None.
> > 
> > Changing the default behavior is IMHO bad anyway does not matter 
> > if Bool
> > or Flag.
> 
> +1
> 
> > 
> > Please advise what is you wish to be implemented :-)
> 
> That depends. Is there a difference between "do not set APPEND_CR 
> ticket 
> flag" and "set APPEND_CR ticket flag to false"?

For YubiKey hardware the flag is either present (true) or absent 
(false). This flag controls whether or not the carriage return is sent 
(present) or not (absent).

Nathaniel

-- 
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 0029] suppress errors arising from deleting non-existent files during client uninstall

2015-04-17 Thread Martin Babinsky

On 04/17/2015 12:36 PM, Martin Basti wrote:

On 17/04/15 12:33, Martin Babinsky wrote:

On 04/17/2015 12:04 PM, Martin Basti wrote:

On 15/04/15 15:53, Martin Babinsky wrote:

On 04/14/2015 04:24 PM, Martin Basti wrote:

On 14/04/15 16:12, Martin Basti wrote:

On 14/04/15 14:25, Martin Babinsky wrote:

This patch addresses https://fedorahosted.org/freeipa/ticket/4966

The noise during rollback/uninstall is caused mainly by unsuccessful
attempts to remove files that do not exist anymore. These errors are
now logged at debug level and do not pop-up to stdout/stderr.




Hello, thank you for the patch.

1)
The option add_warning is quite unclear to me. It does not show
warning but error. I suggest something like,  show_hint,
show_user_action, or something show_additional_..., or
promt_manual_removal

Martin^2



Continue...

2)

 if file_exists(preferences_fname):
 try:
 os.remove(preferences_fname)
 except OSError as e:
 log_file_removal_error(e, preferences_fname,
True)

In this case file not found error should never happen.

Could you remove the 'if file_exists' part and handle just exception?


I just reverted this bit to original form in order to not fix
something that isn't broken. Is that ok?

3)
this is inconsistent with change above, choose one style please:

 if os.path.exists(ca_file):
 try:
 os.unlink(ca_file)
 except OSError, e:
 root_logger.error(
 "Failed to remove '%s': %s", ca_file, e)

--
Martin Basti



Attaching updated patch.


thanks,

just one nitpick, can you move the new function into installutils, it
can be used in different scripts not just in ipaclient.



I'm not sure if it is a good idea as installutils is a part for
freeipa-server package.

Placing it there would create an unnecessary dependency of
freeipa-client on freeipa-server because of a single function.


you are right, I do not why I thought that ipa-client-install uses
installutils.

ACK


self-NACK, I will try to rewrite the patch in a slightly less dumb way.

Sorry for the confusion.

--
Martin^3 Babinsky

--
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 0029] suppress errors arising from deleting non-existent files during client uninstall

2015-04-17 Thread Martin Basti

On 17/04/15 12:33, Martin Babinsky wrote:

On 04/17/2015 12:04 PM, Martin Basti wrote:

On 15/04/15 15:53, Martin Babinsky wrote:

On 04/14/2015 04:24 PM, Martin Basti wrote:

On 14/04/15 16:12, Martin Basti wrote:

On 14/04/15 14:25, Martin Babinsky wrote:

This patch addresses https://fedorahosted.org/freeipa/ticket/4966

The noise during rollback/uninstall is caused mainly by unsuccessful
attempts to remove files that do not exist anymore. These errors are
now logged at debug level and do not pop-up to stdout/stderr.




Hello, thank you for the patch.

1)
The option add_warning is quite unclear to me. It does not show
warning but error. I suggest something like,  show_hint,
show_user_action, or something show_additional_..., or
promt_manual_removal

Martin^2



Continue...

2)

 if file_exists(preferences_fname):
 try:
 os.remove(preferences_fname)
 except OSError as e:
 log_file_removal_error(e, preferences_fname, 
True)


In this case file not found error should never happen.

Could you remove the 'if file_exists' part and handle just exception?


I just reverted this bit to original form in order to not fix
something that isn't broken. Is that ok?

3)
this is inconsistent with change above, choose one style please:

 if os.path.exists(ca_file):
 try:
 os.unlink(ca_file)
 except OSError, e:
 root_logger.error(
 "Failed to remove '%s': %s", ca_file, e)

--
Martin Basti



Attaching updated patch.


thanks,

just one nitpick, can you move the new function into installutils, it
can be used in different scripts not just in ipaclient.



I'm not sure if it is a good idea as installutils is a part for 
freeipa-server package.


Placing it there would create an unnecessary dependency of 
freeipa-client on freeipa-server because of a single function.


you are right, I do not why I thought that ipa-client-install uses 
installutils.


ACK

--
Martin Basti

--
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 0029] suppress errors arising from deleting non-existent files during client uninstall

2015-04-17 Thread Martin Babinsky

On 04/17/2015 12:04 PM, Martin Basti wrote:

On 15/04/15 15:53, Martin Babinsky wrote:

On 04/14/2015 04:24 PM, Martin Basti wrote:

On 14/04/15 16:12, Martin Basti wrote:

On 14/04/15 14:25, Martin Babinsky wrote:

This patch addresses https://fedorahosted.org/freeipa/ticket/4966

The noise during rollback/uninstall is caused mainly by unsuccessful
attempts to remove files that do not exist anymore. These errors are
now logged at debug level and do not pop-up to stdout/stderr.




Hello, thank you for the patch.

1)
The option add_warning is quite unclear to me. It does not show
warning but error. I suggest something like,  show_hint,
show_user_action, or something show_additional_..., or
promt_manual_removal

Martin^2



Continue...

2)

 if file_exists(preferences_fname):
 try:
 os.remove(preferences_fname)
 except OSError as e:
 log_file_removal_error(e, preferences_fname, True)

In this case file not found error should never happen.

Could you remove the 'if file_exists' part and handle just exception?


I just reverted this bit to original form in order to not fix
something that isn't broken. Is that ok?

3)
this is inconsistent with change above, choose one style please:

 if os.path.exists(ca_file):
 try:
 os.unlink(ca_file)
 except OSError, e:
 root_logger.error(
 "Failed to remove '%s': %s", ca_file, e)

--
Martin Basti



Attaching updated patch.


thanks,

just one nitpick, can you move the new function into installutils, it
can be used in different scripts not just in ipaclient.



I'm not sure if it is a good idea as installutils is a part for 
freeipa-server package.


Placing it there would create an unnecessary dependency of 
freeipa-client on freeipa-server because of a single function.


--
Martin^3 Babinsky

--
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 0029] suppress errors arising from deleting non-existent files during client uninstall

2015-04-17 Thread Martin Basti

On 15/04/15 15:53, Martin Babinsky wrote:

On 04/14/2015 04:24 PM, Martin Basti wrote:

On 14/04/15 16:12, Martin Basti wrote:

On 14/04/15 14:25, Martin Babinsky wrote:

This patch addresses https://fedorahosted.org/freeipa/ticket/4966

The noise during rollback/uninstall is caused mainly by unsuccessful
attempts to remove files that do not exist anymore. These errors are
now logged at debug level and do not pop-up to stdout/stderr.




Hello, thank you for the patch.

1)
The option add_warning is quite unclear to me. It does not show
warning but error. I suggest something like,  show_hint,
show_user_action, or something show_additional_..., or
promt_manual_removal

Martin^2



Continue...

2)

 if file_exists(preferences_fname):
 try:
 os.remove(preferences_fname)
 except OSError as e:
 log_file_removal_error(e, preferences_fname, True)

In this case file not found error should never happen.

Could you remove the 'if file_exists' part and handle just exception?

I just reverted this bit to original form in order to not fix 
something that isn't broken. Is that ok?

3)
this is inconsistent with change above, choose one style please:

 if os.path.exists(ca_file):
 try:
 os.unlink(ca_file)
 except OSError, e:
 root_logger.error(
 "Failed to remove '%s': %s", ca_file, e)

--
Martin Basti



Attaching updated patch.


thanks,

just one nitpick, can you move the new function into installutils, it 
can be used in different scripts not just in ipaclient.


--
Martin Basti

--
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] design review: Certificate Profiles

2015-04-17 Thread Jan Cholasta

Dne 17.4.2015 v 10:58 Petr Vobornik napsal(a):

On 04/17/2015 10:12 AM, Fraser Tweedale wrote:

On Fri, Apr 17, 2015 at 10:03:45AM +0200, Jan Cholasta wrote:

Dne 17.4.2015 v 09:45 Fraser Tweedale napsal(a):

On Fri, Apr 17, 2015 at 07:26:55AM +0200, David Kupka wrote:

On 04/16/2015 10:03 AM, Fraser Tweedale wrote:

Hi everyone,

Please review my Certificate Profiles design proposal:
http://www.freeipa.org/page/V4/Certificate_Profiles





5.2.1 CLI - I would change the command to 'ipa certprofile-add' to
stay
consistent with rest of FreeIPA commands.


David, thanks for your input.  'certprofile-import' was chosen after
discussion with Honza, based on the fact the profile already exists
(as a file) and is being imported into the system.  Jan, do you
still agree with '-import'?  What do other people think?


Yes, it should be -import. -add is reserved for the case when the
properties
of the profile are specified directly in command arguments, but in
-import
they are read from the supplied file.


OK, -import it stays; thanks!



Wrt terminology. You might be interested in this PatternFly effort:

https://www.redhat.com/archives/patternfly/2015-March/msg5.html

Interesting part related to topic is:

Add: Add an existing item to an existing list, group, view, or other
container element

Create: Create something new

Import is mentioned later in the thread.

What does it mean for us? There is an effort to unify terminology among
various applications. Some terminology is different from the one in IPA.
We should be aware of that and probably take some steps to standardize
it in a future. E.g. we could start with Web UI. Changing api is a bit
more difficult and I'm not sure if it's even the right thing to do.


It definitely is not the right thing to do. PatternFly has nothing to do 
with the API.


--
Jan Cholasta

--
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] design review: Certificate Profiles

2015-04-17 Thread Petr Vobornik

On 04/17/2015 10:12 AM, Fraser Tweedale wrote:

On Fri, Apr 17, 2015 at 10:03:45AM +0200, Jan Cholasta wrote:

Dne 17.4.2015 v 09:45 Fraser Tweedale napsal(a):

On Fri, Apr 17, 2015 at 07:26:55AM +0200, David Kupka wrote:

On 04/16/2015 10:03 AM, Fraser Tweedale wrote:

Hi everyone,

Please review my Certificate Profiles design proposal:
http://www.freeipa.org/page/V4/Certificate_Profiles





5.2.1 CLI - I would change the command to 'ipa certprofile-add' to stay
consistent with rest of FreeIPA commands.


David, thanks for your input.  'certprofile-import' was chosen after
discussion with Honza, based on the fact the profile already exists
(as a file) and is being imported into the system.  Jan, do you
still agree with '-import'?  What do other people think?


Yes, it should be -import. -add is reserved for the case when the properties
of the profile are specified directly in command arguments, but in -import
they are read from the supplied file.


OK, -import it stays; thanks!



Wrt terminology. You might be interested in this PatternFly effort:

https://www.redhat.com/archives/patternfly/2015-March/msg5.html

Interesting part related to topic is:

Add: Add an existing item to an existing list, group, view, or other 
container element


Create: Create something new

Import is mentioned later in the thread.

What does it mean for us? There is an effort to unify terminology among 
various applications. Some terminology is different from the one in IPA. 
We should be aware of that and probably take some steps to standardize 
it in a future. E.g. we could start with Web UI. Changing api is a bit 
more difficult and I'm not sure if it's even the right thing to do.

--
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] design review: Certificate Profiles

2015-04-17 Thread Fraser Tweedale
On Fri, Apr 17, 2015 at 10:03:45AM +0200, Jan Cholasta wrote:
> Dne 17.4.2015 v 09:45 Fraser Tweedale napsal(a):
> >On Fri, Apr 17, 2015 at 07:26:55AM +0200, David Kupka wrote:
> >>On 04/16/2015 10:03 AM, Fraser Tweedale wrote:
> >>>Hi everyone,
> >>>
> >>>Please review my Certificate Profiles design proposal:
> >>>http://www.freeipa.org/page/V4/Certificate_Profiles
> >>>
> >>>Let me know what is unclear, what needs expansion, and what is plain
> >>>wrong :)
> >>>
> >>>The schema for storing multiple certificates for a principal is
> >>>still being discussed but I expect it will be agreed soon, and I
> >>>will add it to the document.
> >>>
> >>>I am revising the sub-CAs design proposal and it will soon be
> >>>published for review as well.
> >>>
> >>>Cheers,
> >>>Fraser
> >>>
> >>Hi Fraser,
> >>I've read the design page and even though I know only a little about Dogtag
> >>it makes sense to me.
> >>
> >>Just a few notes:
> >>
> >>3.4 Retrieve profile - There was XML format twice (probably
> >>copy-paste-forget to modify :-) I changed it, feel free to revert the change
> >>if it was intentional.
> >>
> >>3.5 Delete profile - IMO the profile should be deleted when user requests
> >>that. If the profile must be disabled before deleted do it as a part of
> >>deletion.
> >>
> >>3.6 Enable/disable profile - When user request enabling/disabling of already
> >>enabled/disabled profile there is no need to return an error. User wants it
> >>to be enabled/disabled and it is, job done.
> 
> Actually not, we raise AlreadyActive/AlreadyInactive in this case (see e.g.
> selinuxusermap-enable).
> 
Good to know - I haven't learned about the SELinux bits yet and
probably wouldn't have found this.

> >>
> >>5.2.1 CLI - I would change the command to 'ipa certprofile-add' to stay
> >>consistent with rest of FreeIPA commands.
> >>
> >David, thanks for your input.  'certprofile-import' was chosen after
> >discussion with Honza, based on the fact the profile already exists
> >(as a file) and is being imported into the system.  Jan, do you
> >still agree with '-import'?  What do other people think?
> 
> Yes, it should be -import. -add is reserved for the case when the properties
> of the profile are specified directly in command arguments, but in -import
> they are read from the supplied file.
> 
OK, -import it stays; thanks!

> >
> >Cheers,
> >Fraser
> >
> 
> 
> -- 
> Jan Cholasta

-- 
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] design review: Certificate Profiles

2015-04-17 Thread Jan Cholasta

Dne 17.4.2015 v 09:45 Fraser Tweedale napsal(a):

On Fri, Apr 17, 2015 at 07:26:55AM +0200, David Kupka wrote:

On 04/16/2015 10:03 AM, Fraser Tweedale wrote:

Hi everyone,

Please review my Certificate Profiles design proposal:
http://www.freeipa.org/page/V4/Certificate_Profiles

Let me know what is unclear, what needs expansion, and what is plain
wrong :)

The schema for storing multiple certificates for a principal is
still being discussed but I expect it will be agreed soon, and I
will add it to the document.

I am revising the sub-CAs design proposal and it will soon be
published for review as well.

Cheers,
Fraser


Hi Fraser,
I've read the design page and even though I know only a little about Dogtag
it makes sense to me.

Just a few notes:

3.4 Retrieve profile - There was XML format twice (probably
copy-paste-forget to modify :-) I changed it, feel free to revert the change
if it was intentional.

3.5 Delete profile - IMO the profile should be deleted when user requests
that. If the profile must be disabled before deleted do it as a part of
deletion.

3.6 Enable/disable profile - When user request enabling/disabling of already
enabled/disabled profile there is no need to return an error. User wants it
to be enabled/disabled and it is, job done.


Actually not, we raise AlreadyActive/AlreadyInactive in this case (see 
e.g. selinuxusermap-enable).




5.2.1 CLI - I would change the command to 'ipa certprofile-add' to stay
consistent with rest of FreeIPA commands.


David, thanks for your input.  'certprofile-import' was chosen after
discussion with Honza, based on the fact the profile already exists
(as a file) and is being imported into the system.  Jan, do you
still agree with '-import'?  What do other people think?


Yes, it should be -import. -add is reserved for the case when the 
properties of the profile are specified directly in command arguments, 
but in -import they are read from the supplied file.




Cheers,
Fraser




--
Jan Cholasta

--
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] design review: Certificate Profiles

2015-04-17 Thread Fraser Tweedale
On Fri, Apr 17, 2015 at 07:26:55AM +0200, David Kupka wrote:
> On 04/16/2015 10:03 AM, Fraser Tweedale wrote:
> >Hi everyone,
> >
> >Please review my Certificate Profiles design proposal:
> >http://www.freeipa.org/page/V4/Certificate_Profiles
> >
> >Let me know what is unclear, what needs expansion, and what is plain
> >wrong :)
> >
> >The schema for storing multiple certificates for a principal is
> >still being discussed but I expect it will be agreed soon, and I
> >will add it to the document.
> >
> >I am revising the sub-CAs design proposal and it will soon be
> >published for review as well.
> >
> >Cheers,
> >Fraser
> >
> Hi Fraser,
> I've read the design page and even though I know only a little about Dogtag
> it makes sense to me.
> 
> Just a few notes:
> 
> 3.4 Retrieve profile - There was XML format twice (probably
> copy-paste-forget to modify :-) I changed it, feel free to revert the change
> if it was intentional.
> 
> 3.5 Delete profile - IMO the profile should be deleted when user requests
> that. If the profile must be disabled before deleted do it as a part of
> deletion.
> 
> 3.6 Enable/disable profile - When user request enabling/disabling of already
> enabled/disabled profile there is no need to return an error. User wants it
> to be enabled/disabled and it is, job done.
> 
> 5.2.1 CLI - I would change the command to 'ipa certprofile-add' to stay
> consistent with rest of FreeIPA commands.
> 
David, thanks for your input.  'certprofile-import' was chosen after
discussion with Honza, based on the fact the profile already exists
(as a file) and is being imported into the system.  Jan, do you
still agree with '-import'?  What do other people think?

Cheers,
Fraser

-- 
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