date:20111103

Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib

2011-11-03 Thread Martin Kosek

On Wed, 2011-11-02 at 18:38 +0200, Alexander Bokovoy wrote:
 (actual patch attached!)
 
 On Wed, 02 Nov 2011, Alexander Bokovoy wrote:
  On Wed, 02 Nov 2011, Jan Cholasta wrote:
   Callable instances are a consequence of the above --
   Command.__call__() does use properties that are changed due to
   finalize() being run. In fact, there are so many places in FreeIPA
   where we simply expect that foo.args/params/output_params/output is
   both iterable and callable instance that it is not even funny.
   
   Now, the process established by the proposed patch is actually
   dependent only on the fact that bound instance has finalize() method.
   This is quite generic and can be used for all other types of objects.
   
   
   The real problem is in the API class so it should be fixed there,
   not worked around in Plugin subclasses.
  Please explain why you do think real problem is in API class. 
  Finalize() is needed purely to being able to split plugins' property 
  initialization into stages before loading plugins and after due to the 
  fact that each and every plugin could contribute commands and 
  additions to the same objects. While these stages are separate, 
  plugins can access properties and commands they want and it is duty of 
  a plugin to get its property working right. We have finalize() method 
  exactly for this purpose.
  
  API class' attempt to call finalize() on each plugin's instance at 
  once is a poor replacement to detecting access to 
  not-yet-initialized properties. We can implement that access like you 
  have proposed with __getattribute__ but it has additional cost for all 
  other properties that don't need post-initialization (finalization) 
  and, what's more important, they incur these costs irrespective 
  whether initialization has happened or not. That's bad and my patch 
  shows it is actually noticeable as the performance difference, for 
  example, of 'ipa dnsrecord-find example.com' between the two patches 
  is about 2x in favor of my patch (1.624s vs 0.912s).
  
  Regarding other objects, here is the list of places where we define 
  specific finalizers:
  $ git grep 'def finalize'
  ipalib/cli.py:def finalize(self):
  ipalib/frontend.py:def finalize(self):
  ipalib/plugable.py:def finalize(self):
  ipalib/plugable.py:def finalize(self):
  ipaserver/rpcserver.py:def finalize(self):
  ipaserver/rpcserver.py:def finalize(self):
  ipaserver/rpcserver.py:def finalize(self):
  tests/test_ipalib/test_cli.py:def finalize(self):
  tests/test_ipalib/test_config.py:def finalize_core(self, ctx, 
  **defaults):
  tests/util.py:def finalize(self, *plugins, **kw):
  
  As you can see, there NO plugins that define their own finalizers, 
  thus, all of them rely on API.finalize(), help.finalize(), and 
  Plugin.finalize()/Command.finalize(). ipaserver/rpcserver.py 
  finalizers are fine as well, I have checked them.
  
  Updated patch is attached. It addresses all comments from Martin.
  
  [root@vm-114 freeipa-2-1-speedup]# time ipa dnsrecord-find ipa.local 
  /dev/null
  
  real0m0.883s
  user0m0.504s
  sys 0m0.136s
  [root@vm-114 freeipa-2-1-speedup]# time ipa user-find /dev/null
  
  real0m0.887s
  user0m0.486s
  sys 0m0.141s
  [root@vm-114 freeipa-2-1-speedup]# time ipa group-find /dev/null
  
  real0m0.933s
  user0m0.446s
  sys 0m0.169s
  [root@vm-114 freeipa-2-1-speedup]# time ipa help /dev/null
  
  real0m0.624s
  user0m0.479s
  sys 0m0.133s
  [root@vm-114 freeipa-2-1-speedup]# time ipa group /dev/null
  
  real0m0.612s
  user0m0.475s
  sys 0m0.126s
  [root@vm-114 freeipa-2-1-speedup]# time ipa user /dev/null
  
  real0m0.617s
  user0m0.472s
  sys 0m0.131s
  
  -- 
  / Alexander Bokovoy
 

Good, this indeed addresses all my previous concerns but one :-)

$ ./makeapi 
Writing API to API.txt
Traceback (most recent call last):
  File ./makeapi, line 392, in module
sys.exit(main())
  File ./makeapi, line 376, in main
rval |= make_api()
  File ./makeapi, line 167, in make_api
fd.write('args: %d,%d,%d\n' % (len(cmd.args), len(cmd.options), 
len(cmd.output)))

But if you change len functions to __len__ it works fine.

Martin


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

Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib

2011-11-03 Thread Alexander Bokovoy

On Thu, 03 Nov 2011, Martin Kosek wrote:
 Good, this indeed addresses all my previous concerns but one :-)
 
 $ ./makeapi 
 Writing API to API.txt
 Traceback (most recent call last):
   File ./makeapi, line 392, in module
 sys.exit(main())
   File ./makeapi, line 376, in main
 rval |= make_api()
   File ./makeapi, line 167, in make_api
 fd.write('args: %d,%d,%d\n' % (len(cmd.args), len(cmd.options), 
 len(cmd.output)))
 
 But if you change len functions to __len__ it works fine.
I suspected this. :)
Ok, that and I protected self.__finalized reassignment in case 
Plugin#finalize() got called twice -- second time the class is locked 
already so self.__finalized = True will blow exception. I made it 
no-op for next passes.

New patch attached. Survived fresh re-install.
-- 
/ Alexander Bokovoy
From 050e75b18a2b6856d0626edbdcabed10aa841ad3 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy aboko...@redhat.com
Date: Wed, 2 Nov 2011 12:24:20 +0200
Subject: [PATCH] Perform late initialization of FreeIPA plugins

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

When plugins are loaded, instances of the provided objects and commands
are registered in the API instance. The patch changes finalization process
to apply on first access to the Command instance that would cause either
access to properties (args, options, params) or execution of the command
itself.

The patch gives 2x boost for client-side commands like help and 3x boost
for commands that go to the server side. All performance numbers are
approximate and may vary a lot.
---
 ipalib/frontend.py |   50 
 ipalib/plugable.py |   13 ++---
 tests/test_ipalib/test_frontend.py |7 +++--
 3 files changed, 63 insertions(+), 7 deletions(-)

diff --git a/ipalib/frontend.py b/ipalib/frontend.py
index 
61e7f493f8a8e30a1a189d06cd6a69893319deaf..45d254be23d6c8b0c359c8a4e84d0658a5bf4fe7
 100644
--- a/ipalib/frontend.py
+++ b/ipalib/frontend.py
@@ -64,6 +64,44 @@ def entry_count(entry):
 
 return num_entries
 
+class _MirrorTrap(object):
+
+_MirrorTrap is a special class to support late initialization in FreeIPA.
+_MirrorTrap instances can be used instead of None for properties that
+change their state due to Plugin#finalize() method called.
+
+As Plugin#finalize() might be computationally expensive, objects that 
could not
+act without such properties, should initialize them to _MirrorTrap() 
instance
+in their __init__() method so that _MirrorTrap instance would force the 
object
+to finalize() before returning the value of the property.
+
+Usage pattern is following:
+   self.args = _MirrorTrap(self, 'args')
+
+Pass the reference to the object holding the attribute and the name of the 
attribute.
+On first access to the attribute, _MirrorTrip will try to call object's 
finalize() method
+and expects that the attribute will be re-assigned with new (proper) value.
+
+In many places in FreeIPA, an attribute gets assigned with NameSpace() 
instance during 
+finalize() call.
+
+def __init__(self, master, attr):
+self.master = master
+self.attr = attr
+
+def __call__(self, **args):
+self.master.finalize()
+# At this point master.attr points to proper object
+return getattr(self.master, self.attr)(**args)
+
+def __iter__(self):
+self.master.finalize()
+return getattr(self.master, self.attr).__iter__()
+
+def __len__(self):
+self.master.finalize()
+return getattr(self.master, self.attr).__len__()
+
 
 class HasParam(Plugin):
 
@@ -404,6 +442,14 @@ class Command(HasParam):
 msg_summary = None
 msg_truncated = _('Results are truncated, try a more specific search')
 
+def __init__(self):
+self.args = _MirrorTrap(self, 'args')
+self.options = _MirrorTrap(self, 'options')
+self.output_params = _MirrorTrap(self, 'output_params')
+self.output = _MirrorTrap(self, 'output')
+
+super(Command, self).__init__()
+
 def __call__(self, *args, **options):
 
 Perform validation and then execute the command.
@@ -411,6 +457,10 @@ class Command(HasParam):
 If not in a server context, the call will be forwarded over
 XML-RPC and the executed an the nearest IPA server.
 
+# Plugin instance must be finalized before we get to execution
+if not self.__dict__['_Plugin__finalized']:
+self.finalize()
+
 params = self.args_options_2_params(*args, **options)
 self.debug(
 'raw: %s(%s)', self.name, ', '.join(self._repr_iter(**params))
diff --git a/ipalib/plugable.py b/ipalib/plugable.py
index 
b0e415656e0428eb164c35a2862fcfbf50883381..d1411de32fc1a888db35e1dafdf6bf4fe8d0634b
 100644
--- a/ipalib/plugable.py
+++ b/ipalib/plugable.py
@@ -207,6 +207,7 @@ class Plugin(ReadOnly):
 self.label

Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib

2011-11-03 Thread Jan Cholasta


Dne 2.11.2011 17:38, Alexander Bokovoy napsal(a):

(actual patch attached!)

On Wed, 02 Nov 2011, Alexander Bokovoy wrote:

On Wed, 02 Nov 2011, Jan Cholasta wrote:

Callable instances are a consequence of the above --
Command.__call__() does use properties that are changed due to
finalize() being run. In fact, there are so many places in FreeIPA
where we simply expect that foo.args/params/output_params/output is
both iterable and callable instance that it is not even funny.

Now, the process established by the proposed patch is actually
dependent only on the fact that bound instance has finalize() method.
This is quite generic and can be used for all other types of objects.



The real problem is in the API class so it should be fixed there,
not worked around in Plugin subclasses.

Please explain why you do think real problem is in API class.
Finalize() is needed purely to being able to split plugins' property
initialization into stages before loading plugins and after due to the
fact that each and every plugin could contribute commands and
additions to the same objects. While these stages are separate,
plugins can access properties and commands they want and it is duty of
a plugin to get its property working right. We have finalize() method
exactly for this purpose.

API class' attempt to call finalize() on each plugin's instance at
once is a poor replacement to detecting access to
not-yet-initialized properties. We can implement that access like you
have proposed with __getattribute__ but it has additional cost for all
other properties that don't need post-initialization (finalization)
and, what's more important, they incur these costs irrespective
whether initialization has happened or not. That's bad and my patch
shows it is actually noticeable as the performance difference, for
example, of 'ipa dnsrecord-find example.com' between the two patches
is about 2x in favor of my patch (1.624s vs 0.912s).

Regarding other objects, here is the list of places where we define
specific finalizers:
$ git grep 'def finalize'
ipalib/cli.py:def finalize(self):
ipalib/frontend.py:def finalize(self):
ipalib/plugable.py:def finalize(self):
ipalib/plugable.py:def finalize(self):
ipaserver/rpcserver.py:def finalize(self):
ipaserver/rpcserver.py:def finalize(self):
ipaserver/rpcserver.py:def finalize(self):
tests/test_ipalib/test_cli.py:def finalize(self):
tests/test_ipalib/test_config.py:def finalize_core(self, ctx, **defaults):
tests/util.py:def finalize(self, *plugins, **kw):

As you can see, there NO plugins that define their own finalizers,
thus, all of them rely on API.finalize(), help.finalize(), and
Plugin.finalize()/Command.finalize(). ipaserver/rpcserver.py
finalizers are fine as well, I have checked them.

Updated patch is attached. It addresses all comments from Martin.

[root@vm-114 freeipa-2-1-speedup]# time ipa dnsrecord-find ipa.local/dev/null

real0m0.883s
user0m0.504s
sys 0m0.136s
[root@vm-114 freeipa-2-1-speedup]# time ipa user-find/dev/null

real0m0.887s
user0m0.486s
sys 0m0.141s
[root@vm-114 freeipa-2-1-speedup]# time ipa group-find/dev/null

real0m0.933s
user0m0.446s
sys 0m0.169s
[root@vm-114 freeipa-2-1-speedup]# time ipa help/dev/null

real0m0.624s
user0m0.479s
sys 0m0.133s
[root@vm-114 freeipa-2-1-speedup]# time ipa group/dev/null

real0m0.612s
user0m0.475s
sys 0m0.126s
[root@vm-114 freeipa-2-1-speedup]# time ipa user/dev/null

real0m0.617s
user0m0.472s
sys 0m0.131s

--
/ Alexander Bokovoy




The problem with the API class is that it creates all the plugin 
instances every time. Both mine and your patch don't change that, they 
deal only with finalization, which is the easier part of the lazy 
initialization problem. Additionally, in your patch finalize() is called 
only on Command subclasses, so non-Command plugins are not 
ReadOnly-locked like they should (currently this is done only when 
production mode is off).


We could change API so that the plugins are initialized on-demand. That 
way we could probably get rid off finalize() on plugins altogether and 
move the initialization code into __init__() (because all the required 
information would be available on-demand) and the locking into API. The 
flaw of this approach is that it would require a number of fundamental 
changes, namely changing the way API class handles plugin initialization 
and moving the initialization of static Plugin properties (name, 
module, fullname, bases, label, ...) from instance level to class level. 
(Nonetheless, I think API would't suffer from a facelift - currently it 
is messy, hard to read code, with bits nobody ever used or uses anymore, 
some of the docstrings and comments aren't correct, etc.)


Anyway, if we decide to go with your solution, please make sure *all* 
the plugins that are used are finalized (because of the locking) and add 
a new api.env attribute which controls the lazy

[Freeipa-devel] [PATCH] 158 Fix ipa-server-install answer cache

2011-11-03 Thread Martin Kosek

Current Answer Cache storing mechanism is not ideal for storing
non-trivial Python types like arrays, custom classes, etc.
RawConfigParser just translates values to string, which
are not correctly decoded when the Answer Cache is parsed and
restored in the installer.

This patch replaces RawConfigParser with Python's standard pickle
module, which is a recommended way for serialization in Python.

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

From a2c9b9f87050760c53ddff358a1cb5609f2c5a4e Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Thu, 3 Nov 2011 11:08:26 +0100
Subject: [PATCH] Fix ipa-server-install answer cache

Current Answer Cache storing mechanism is not ideal for storing
non-trivial Python types like arrays, custom classes, etc.
RawConfigParser just translates values to string, which
are not correctly decoded when the Answer Cache is parsed and
restored in the installer.

This patch replaces RawConfigParser with Python's standard pickle
module, which is a recommended way for serialization in Python.

https://fedorahosted.org/freeipa/ticket/2054
---
 install/tools/ipa-server-install |   65 +++--
 1 files changed, 26 insertions(+), 39 deletions(-)

diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index d29b806da4807531f8907229eefa783f0d570f08..1dbeef59620ff135efd68fb47fef740015b62639 100755
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -36,7 +36,7 @@ import signal
 import shutil
 import glob
 import traceback
-from ConfigParser import RawConfigParser
+import pickle
 import random
 import tempfile
 import nss.error
@@ -302,45 +302,36 @@ ANSWER_CACHE = /root/.ipa_cache
 
 def read_cache(dm_password):
 
-Returns a dict of cached answers or None if no cache file exists.
+Returns a dict of cached answers or empty dict if no cache file exists.
 
 if not ipautil.file_exists(ANSWER_CACHE):
 return {}
 
 top_dir = tempfile.mkdtemp(ipa)
+fname = %s/cache % top_dir
 try:
-clearfile = %s/cache % top_dir
-decrypt_file(ANSWER_CACHE, clearfile, dm_password, top_dir)
+decrypt_file(ANSWER_CACHE, fname, dm_password, top_dir)
 except Exception, e:
 shutil.rmtree(top_dir)
-raise RuntimeError(Problem decrypting answer cache in %s, check your password. % ANSWER_CACHE)
+raise Exception(Decryption of answer cache in %s failed, please check your password. % ANSWER_CACHE)
 
-optdict={}
-parser = RawConfigParser()
 try:
-fp = open(clearfile, r)
-parser.readfp(fp)
-optlist = parser.items('options')
-fp.close()
-
+with open(fname, 'rb') as f:
+try:
+optdict = pickle.load(f)
+except Exception, e:
+raise Exception(Parse error in %s: %s % (ANSWER_CACHE, str(e)))
 except IOError, e:
-raise RuntimeError(Error reading cache file %s: %s % (ANSWER_CACHE, str(e)))
+raise Exception(Read error in %s: %s % (ANSWER_CACHE, str(e)))
 finally:
 shutil.rmtree(top_dir)
 
-for opt in optlist:
-value = opt[1]
-if value.lower() in ['true', 'false']:
-value = value.lower() == 'true'
-if value == 'None':
-value = None
-optdict[opt[0]] = value
-
 # These are the only ones that may be overridden
-if 'external_ca_file' in optdict:
-del optdict['external_ca_file']
-if 'external_cert_file' in optdict:
-del optdict['external_cert_file']
+for opt in ('external_ca_file', 'external_cert_file'):
+try:
+del optdict[opt]
+except KeyError:
+pass
 
 return optdict
 
@@ -348,21 +339,14 @@ def write_cache(options):
 
 Takes a dict as input and writes a cached file of answers
 
-
-# convert the options instance into a dict
-optdict = eval(str(options))
-parser = RawConfigParser()
 top_dir = tempfile.mkdtemp(ipa)
+fname = %s/cache % top_dir
 try:
-fp = open(%s/cache % top_dir, w)
-parser.add_section('options')
-for opt in optdict:
-parser.set('options', opt, optdict[opt])
-parser.write(fp)
-fp.close()
-ipautil.encrypt_file(%s/cache % top_dir, ANSWER_CACHE, options.dm_password, top_dir);
+with open(fname, 'wb') as f:
+pickle.dump(options, f)
+ipautil.encrypt_file(fname, ANSWER_CACHE, options['dm_password'], top_dir)
 except IOError, e:
-raise RuntimeError(Unable to cache command-line options %s % str(e))
+raise Exception(Unable to cache command-line options %s % str(e))
 finally:
 shutil.rmtree(top_dir)
 
@@ -636,7 +620,10 @@ def main():
 dm_password = read_password(Directory Manager, confirm=False)
 if dm_password is None:
 sys.exit(\nDirectory Manager password required)
-

Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib

2011-11-03 Thread Alexander Bokovoy

On Thu, 03 Nov 2011, Jan Cholasta wrote:
 The problem with the API class is that it creates all the plugin
 instances every time. Both mine and your patch don't change that,
 they deal only with finalization, which is the easier part of the
 lazy initialization problem. Additionally, in your patch finalize()
 is called only on Command subclasses, so non-Command plugins are not
 ReadOnly-locked like they should (currently this is done only when
 production mode is off).
I ended up with explicitly finalizing Object, Property, and Backend 
objects. This, by the way, proved that major slowdown is brought by 
the Command and Method instances as I've got statistically negligible 
variations of execution time, within less than 1%.

 We could change API so that the plugins are initialized on-demand.
 That way we could probably get rid off finalize() on plugins
 altogether and move the initialization code into __init__() (because
 all the required information would be available on-demand) and the
 locking into API. The flaw of this approach is that it would require
 a number of fundamental changes, namely changing the way API class
 handles plugin initialization and moving the initialization of
 static Plugin properties (name, module, fullname, bases, label,
 ...) from instance level to class level. (Nonetheless, I think API
 would't suffer from a facelift - currently it is messy, hard to read
 code, with bits nobody ever used or uses anymore, some of the
 docstrings and comments aren't correct, etc.)
A review of API class is a good idea. However, I think it is currently 
enough what is in proposed patch as it gets you 2-3x speed increase 
already.

One important feature I'd like to still keep in FreeIPA is ability to 
extend any object and action during plugin import phase regardless of 
the python source file it comes from. So far our split between 'user 
methods' and 'dnsrecord methods' is mostly utilitarian as they are 
implemented in their own source code files. However, nobody prevents 
you from implementing all needed modifications to all classes in the 
single source file and I expect this to be fairly typical to 
site-specific cases.

This is something I'd like to keep as it has great value for all 
end-users and it requires loading all Python files in ipalib/plugins 
(and in ipaserver/plugins for server side).

 Anyway, if we decide to go with your solution, please make sure
 *all* the plugins that are used are finalized (because of the
 locking) and add a new api.env attribute which controls the lazy
 finalization, so that the behavior can be overriden (actually I have
 already done that - see attached patch - just use
 api.env.plugins_on_demand instead of api.env.context == 'cli').
Done.

-- 
/ Alexander Bokovoy
From 44ebebc2fede6f001a826fa4047abfeb02195cac Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy aboko...@redhat.com
Date: Wed, 2 Nov 2011 12:24:20 +0200
Subject: [PATCH] Perform late initialization of FreeIPA plugins

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

When plugins are loaded, instances of the provided objects and commands
are registered in the API instance. The patch changes finalization process
to apply on first access to the Command instance that would cause either
access to properties (args, options, params) or execution of the command
itself.

The patch gives 2x boost for client-side commands like help and 3x boost
for commands that go to the server side. All performance numbers are
approximate and may vary a lot.
---
 ipalib/config.py   |4 ++
 ipalib/constants.py|1 +
 ipalib/frontend.py |   57 
 ipalib/plugable.py |   28 +++--
 makeapi|1 +
 tests/test_ipalib/test_frontend.py |6 ++--
 6 files changed, 90 insertions(+), 7 deletions(-)

diff --git a/ipalib/config.py b/ipalib/config.py
index 
410e5f0b252fc423e9c673e4f5b62e50eaf3602e..5e3ef8d9bc529dea59890bc8974b1d455113b14c
 100644
--- a/ipalib/config.py
+++ b/ipalib/config.py
@@ -492,6 +492,10 @@ class Env(object):
 if 'conf_default' not in self:
 self.conf_default = self._join('confdir', 'default.conf')
 
+# Set plugins_on_demand:
+if 'plugins_on_demand' not in self:
+self.plugins_on_demand = (self.context == 'cli')
+
 def _finalize_core(self, **defaults):
 
 Complete initialization of standard IPA environment.
diff --git a/ipalib/constants.py b/ipalib/constants.py
index 
6d246288b0bb6ad3509fdb62616a03d678312319..7ec897b58786b5d7047d5fbdafb655b7d71a5400
 100644
--- a/ipalib/constants.py
+++ b/ipalib/constants.py
@@ -188,6 +188,7 @@ DEFAULT_CONFIG = (
 ('confdir', object),  # Directory containing config files
 ('conf', object),  # File containing context specific config
 ('conf_default', object),  # File containing context independent config
+('plugins_on_demand', object),  # Whether to finalize

[Freeipa-devel] [PATCH] 030 Extending facet's mechanism of gathering changes

2011-11-03 Thread Petr Vobornik


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

I'm not sure if update_info and other new classes should be in details.js.
--
Petr Vobornik
From 0506538ec9da347f2d2b7bac103b9c06fc405c2f Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Wed, 2 Nov 2011 16:43:00 +0100
Subject: [PATCH] Extending facet's mechanism of gathering changes

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

Adding option to gathering changes for update from widgets, sections, details facet.

Changes are represented by update_info { fields [] ((field_info)), commands [] ((command_info))  } object.

* On calling get_update_info() method widget, section and facet returns update_info object which represents all changes in nested objects. Thus usually widgets are creating update_infos, their containers are merging them.
* This object can be then used in details facet update method. In order to use it command_mode = 'init' has to be set. Command mode was introduced to support backward compatibility.
* command_info consists of command and priority. Priority can be set to specify exact exectuting order of commands. It can be defined on facet level by setting widget's priority. When widgit is creating command_info it should pas its priority to it.
---
 install/ui/details.js |  347 
 install/ui/ipa.js |   10 +-
 install/ui/widget.js  |   15 ++-
 3 files changed, 310 insertions(+), 62 deletions(-)

diff --git a/install/ui/details.js b/install/ui/details.js
index 15056204f72ef2095862c2c35d24cd47fbc819b3..621d47ecdd95672d530b78c0eb707e4af96002d8 100644
--- a/install/ui/details.js
+++ b/install/ui/details.js
@@ -207,6 +207,20 @@ IPA.details_section = function(spec) {
 }
 };
 
+that.get_update_info = function() {
+
+var update_info = IPA.update_info_builder.new_update_info();
+
+var fields = that.fields.values;
+for(var i=0; i  fields.length; i++) {
+update_info = IPA.update_info_builder.merge(
+update_info,
+fields[i].get_update_info());
+}
+
+return update_info;
+};
+
 init();
 
 // methods that should be invoked by subclasses
@@ -280,6 +294,8 @@ IPA.details_facet = function(spec) {
 that.entity = spec.entity;
 that.pre_execute_hook = spec.pre_execute_hook;
 that.post_update_hook = spec.post_update_hook;
+that.update_command_name = spec.update_command_name || 'mod';
+that.command_mode = spec.command_mode || 'save'; // [save, info]
 
 that.label = spec.label || IPA.messages  IPA.messages.facets  IPA.messages.facets.details;
 that.facet_group = spec.facet_group || 'settings';
@@ -398,11 +414,7 @@ IPA.details_facet = function(spec) {
 if (that.update_button.hasClass('action-button-disabled')) return false;
 
 if (!that.validate()) {
-var dialog = IPA.message_dialog({
-title: IPA.messages.dialogs.validation_title,
-message: IPA.messages.dialogs.validation_message
-});
-dialog.open();
+that.show_validation_error();
 return false;
 }
 
@@ -598,6 +610,7 @@ IPA.details_facet = function(spec) {
 that.enable_update(false);
 };
 
+
 that.validate = function() {
 var valid = true;
 var sections = that.sections.values;
@@ -608,46 +621,40 @@ IPA.details_facet = function(spec) {
 return valid;
 };
 
-that.update = function(on_win, on_fail) {
 
-function on_success(data, text_status, xhr) {
-if (on_win)
-on_win(data, text_status, xhr);
-if (data.error)
-return;
+that.on_update_success = function(data, text_status, xhr) {
 
-if (that.post_update_hook) {
-that.post_update_hook(data, text_status);
-return;
-}
+if (data.error)
+return;
 
-var result = data.result.result;
-that.load(result);
+if (that.post_update_hook) {
+that.post_update_hook(data, text_status);
+return;
 }
 
-function on_error(xhr, text_status, error_thrown) {
-if (on_fail)
-on_fail(xhr, text_status, error_thrown);
-}
+var result = data.result.result;
+that.load(result);
+};
+
+that.on_update_error = function(xhr, text_status, error_thrown) {
+};
 
-var args = that.get_primary_key();
+that.add_fields_to_command = function(update_info, command) {
 
-var command = IPA.command({
-entity: that.entity.name,
-method: 'mod',
-args: args,
-options: {
-all: true,
-rights: true
-},
-on_success: on_success,
-on_error: on_error
-});
+for

Re: [Freeipa-devel] Unifying the PKI and IPA Directory Server instances

2011-11-03 Thread Adam Young


On 11/03/2011 12:56 AM, Simo Sorce wrote:

On Wed, 2011-11-02 at 20:25 -0400, Adam Young wrote:

On 11/02/2011 06:19 PM, Rob Crittenden wrote:

Simo Sorce wrote:

On Wed, 2011-11-02 at 16:44 -0400, Ade Lee wrote:

On Wed, 2011-11-02 at 16:03 -0400, Adam Young wrote:

[...]

So, a user becomes an agent on the ca by having a certificate in the
user record and being a member of the relevant admin, agent or auditor
group.

I see this as follows:
1. ipa cms-user-add (add a user and add the auxilliary cmsuser object
class)
2. ipa user-cert (contact the ca and get a certificate for this user,
add this cert to the user record in the ipa database)
3. ipa group-add-member (add the user to the relevant group)

At no point does PKI need to modify anything in the IPA database.

Sounds reasonable.
Can you post a link to the schema that would be added to IPA objects ?

Simo.


I think this is it:

http://svn.fedorahosted.org/svn/pki/trunk/pki/base/ca/shared/conf/schema.ldif

Look for cmsuser.

Unfortunately it looks like the cmsuser objectclass is of type
structural, which means it cannot be added to existing records.


The cert seems to  comes from

05rfc4523.ldif

and is added in

06inetorgperson.ldif

Which is already in our user record.

CMS only seems to require usertype, which is a string, and allows
userstate  which is an integer.

I wonder if we can convince PKI to use a different schema to reprsent
this information. We can use Roles or Groups to tell what type of user a
user is, not sure about the state as that schema file has exactly the
same comment for both usertype and userstate, seems a bug.


I think so.  I do not know if CMSuser is really needed, as it looks like 
everything in there is accounted for some other way.


My guess is the type  field is used to enforce that someone in one 
group,  say agents, cannot also be in a different group, say auditors 
but they do use groups for  most roles in the system.


I think that there are going to be severl layers for the permissions in 
the IPA approach:  For CAAdmins, we create a group CAdmins, and  add a 
Role to to CAAdminRole which contains the appropriate Permissions.  Same 
for Auditors and agents.  No one should have the ACI to change these roles.


We probably want to put in an RFE for IPA Roles  that state two roles 
are mutually exclusive.



userstate is, I think, to disable an account, which is handled using 
nsaccount lock  in IPA.  I think we should agree on a single mechanism, 
and it should be the one in the most standard schema.









IIRC the user we create in CS now has the description attribute set up
in a very specific way. Is that still required?

What is description used for ?

Simo.




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

Re: [Freeipa-devel] Unifying the PKI and IPA Directory Server instances

2011-11-03 Thread Rob Crittenden


Ade Lee wrote:

On Wed, 2011-11-02 at 16:03 -0400, Adam Young wrote:

To clarify:  there are two types of Data stored in the PKI CA DS
instances.  One is Users and groups (IdM),  and the other is
certificates and requests.

The CA currently administers its own users:  creates, add deletes, add
privs and so forth.  If  we extract the IdM  objects from the CA
control, we will need to build another mechanism to manage them.

The Certificates will stay in their own suffix.  I don't think anyone
disagrees about this.

I'm trying to think through the steps of using the IPA user database for
PKI.  If I undertand the end state, the general flow will be driven from
ipa-server-install and will look like this:

1.  Create a unified DS instance.  We can continue to name it the way
that IPA does:   All caps the hostname.
2.  Apply the LDAP schema LDIFs to it.  At this point, we probably want
to create the whole IPA schema,  and the cmsUser as well


For clarification, cmsuser is just an object that is defined in the PKI
schema, not the actual admin user created in the install itself.

It may be advantageous to actually pre-create this user when applying
schema LDIFS and just have pkisilent add the user certs.


The description attribute needs to store per-instance specific 
information such as the requestId. Unless you mean just adding userstate 
and usertype.



3.  Call PKISilent (or equivalent) passing the info for  the Unified
directory server instance, to include the IPA tree for the users.
4.  PKISilent will create the PKI admin user,  to include its
certificiate in the IPA tree.  This user will be half baked from an IPA
perspective,  but will allow administration of the CA.


Pre-creating this user may solve this problem.  You can pre-create a
user with all the required IPA and PKI attributes and just have
pkisilent add the user cert needed.

As we will be running as directory manager in this case, we will
permissions to do this.


5.  Create a PKIDirectory Manager account that has complete control over
only the PKI  suffix, and not the IPA suffix.  Modify the PKI code to
use only this account.  This account should be able to authenticate
against the Dir Srv using a client certificate, stored in the PKI
specific NSS database.


This of course involves setting up the relevant ACIs.  We may want to
consider the reverse.  That only certain IPA accounts should have access
to the PKI data.


Which still involves ACIs, right?


6.  Restart the CA.
7.  Continue the IPA server install from the.  Fix up the Admin user so
that it works for IPA.


Not needed if we pre-populate as mentioned above.


8.  Disable the Directory Manager's password.  From here on out,  access
to the Dir Srv should be either certificate or Keytab only.


After IPA is up and running, the management of the User Database is done
by IPA.
One thing that several people have asked for in IPA is the ability to
manage external Roles:  ones that don't map to ACIs.  PKI has this
requirement as well.  There are a couple approaches we could take:

1.  Attempt to use the current Role mechanism in IPA.  This gets hidden
to an outside user,  but that should be OK.  Checking if a user is a PKI
Admin, Agent, or Auditor should be done only by a privileged account anyway.

2.  Use a User Group:  PKIAdmins,  PKIAgents,  and PKIAuditors.This
is what I would suggest.

3.  Create an external mechanism.


Once IPA has assumed control of the IdM DB, we will still need to be
able to get user certificates into the  user records CMSUser field.  I
do not know CS well enough to say how that would work,  but I assume it
will be one of these two mechanisms:

1.  Use the CA Administrative interface to modify an existing user to
have the certificate
or
2.  Add a mechanism to IPA to request and apply the certificate to the
IPA User.

I'm guessing that the flow would be something like this:

1.  Create a user  (ipa user-add)
2.  Assign them to the PKIAgents groups
3.  Call the CA Admin interface to make the user an agent.

If we do it this way, the  PKI instance will need to be able to modify
the IPA user record.  Alternatively:

1.  ipa user-add
2.  ipa group-add-member
3.  ipa user-cert


So, a user becomes an agent on the ca by having a certificate in the
user record and being a member of the relevant admin, agent or auditor
group.

I see this as follows:
1. ipa cms-user-add (add a user and add the auxilliary cmsuser object
class)
2. ipa user-cert (contact the ca and get a certificate for this user,
add this cert to the user record in the ipa database)
3. ipa group-add-member (add the user to the relevant group)

At no point does PKI need to modify anything in the IPA database.


And it never should IMHO. We need to maintain the idea that the CA is 
some black box that we can poke at from time to time to get data but I'd 
prefer to keep them discrete.




As an added bonus, we get user certs.


One discussion we've had on the side is about scalability.  As the
Databases

[Freeipa-devel] [PATCH] fix memleak in ipa-kdb

2011-11-03 Thread Simo Sorce

Pushed the attached patch under the oneliner rule.
-- 
Simo Sorce * Red Hat, Inc * New York
From 9f07404fe3f426cb45896dc5e71fbd0492fb8ea3 Mon Sep 17 00:00:00 2001
From: Simo Sorce sso...@redhat.com
Date: Fri, 21 Oct 2011 12:43:30 -0400
Subject: [PATCH] ipa-kdb: Fix memory leak

---
 daemons/ipa-kdb/ipa_kdb.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/daemons/ipa-kdb/ipa_kdb.c b/daemons/ipa-kdb/ipa_kdb.c
index 880a7890b6814290e6139c1685b88aced8ce6867..6a6c2063902f8b2a76d97f3510f09333c5af168d 100644
--- a/daemons/ipa-kdb/ipa_kdb.c
+++ b/daemons/ipa-kdb/ipa_kdb.c
@@ -262,6 +262,7 @@ int ipadb_get_connection(struct ipadb_context *ipactx)
 ret = 0;
 
 done:
+ldap_msgfree(res);
 if (ret) {
 if (ipactx-lcontext) {
 ldap_unbind_ext_s(ipactx-lcontext, NULL, NULL);
-- 
1.7.6.4

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

[Freeipa-devel] [PATCH] 031 Field for DNS SOA class changed to combobox with options

2011-11-03 Thread Petr Vobornik


https://fedorahosted.org/freeipa/ticket/602
--
Petr Vobornik
From 47022d96920b16a81ee54d53725de2e00d8c5c91 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Thu, 3 Nov 2011 15:14:15 +0100
Subject: [PATCH] Field for DNS SOA class changed to combobox with options

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

SOA class is an enumerated field. Changing input field to combobox with options allows inserting only valid value.
---
 install/ui/dns.js |   24 +++-
 1 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/install/ui/dns.js b/install/ui/dns.js
index 4dbf3e0d26699330b18285306ae7f6ee2c377324..769eee6020cd0be217ba64deada312b9a99ab2c9 100644
--- a/install/ui/dns.js
+++ b/install/ui/dns.js
@@ -62,7 +62,13 @@ IPA.entity_factories.dnszone = function() {
 'idnssoaexpire',
 'idnssoaminimum',
 'dnsttl',
-'dnsclass',
+{
+factory: IPA.combobox_widget,
+name: 'dnsclass',
+options: [
+'IN', 'CS', 'CH', 'HS'
+]
+},
 {
 factory: IPA.radio_widget,
 name: 'idnsallowdynupdate',
@@ -435,14 +441,14 @@ IPA.entity_factories.dnsrecord = function() {
 return IPA.entity_builder().
 entity('dnsrecord').
 containing_entity('dnszone').
-details_facet({
+details_facet({
 post_update_hook:function(data){
 var result = data.result.result;
  if (result.idnsname) {
 this.load(result);
 } else {
 this.reset();
-var dialog = IPA.dnsrecord_redirection_dialog();
+var dialog = IPA.dnsrecord_redirection_dialog();
 dialog.open(this.container);
 }
 },
@@ -603,11 +609,11 @@ IPA.entity_factories.dnsrecord = function() {
 };
 
 IPA.dnsrecord_redirection_dialog = function(spec) {
-spec = spec || {};
-spec.title = spec.title || IPA.messages.dialogs.redirection;  
-
-var that = IPA.dialog(spec);
-
+spec = spec || {};
+spec.title = spec.title || IPA.messages.dialogs.redirection;
+
+var that = IPA.dialog(spec);
+
 that.create = function() {
 $('p/', {
 'text': IPA.messages.objects.dnsrecord.deleted_no_data
@@ -616,7 +622,7 @@ IPA.dnsrecord_redirection_dialog = function(spec) {
 'text': IPA.messages.objects.dnsrecord.redirection_dnszone
 }).appendTo(that.container);
 };
-
+
 that.create_button({
 name: 'ok',
 label: IPA.messages.buttons.ok,
-- 
1.7.6.4

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

Re: [Freeipa-devel] Unifying the PKI and IPA Directory Server instances

2011-11-03 Thread Ade Lee

On Thu, 2011-11-03 at 09:20 -0400, Adam Young wrote:
 On 11/03/2011 12:56 AM, Simo Sorce wrote:
  On Wed, 2011-11-02 at 20:25 -0400, Adam Young wrote:
  On 11/02/2011 06:19 PM, Rob Crittenden wrote:
  Simo Sorce wrote:
  On Wed, 2011-11-02 at 16:44 -0400, Ade Lee wrote:
  On Wed, 2011-11-02 at 16:03 -0400, Adam Young wrote:
  [...]
  So, a user becomes an agent on the ca by having a certificate in the
  user record and being a member of the relevant admin, agent or auditor
  group.
 
  I see this as follows:
  1. ipa cms-user-add (add a user and add the auxilliary cmsuser object
  class)
  2. ipa user-cert (contact the ca and get a certificate for this user,
  add this cert to the user record in the ipa database)
  3. ipa group-add-member (add the user to the relevant group)
 
  At no point does PKI need to modify anything in the IPA database.
  Sounds reasonable.
  Can you post a link to the schema that would be added to IPA objects ?
 
  Simo.
 
  I think this is it:
 
  http://svn.fedorahosted.org/svn/pki/trunk/pki/base/ca/shared/conf/schema.ldif
 
  Look for cmsuser.
  Unfortunately it looks like the cmsuser objectclass is of type
  structural, which means it cannot be added to existing records.
 
  The cert seems to  comes from
 
  05rfc4523.ldif
 
  and is added in
 
  06inetorgperson.ldif
 
  Which is already in our user record.
 
  CMS only seems to require usertype, which is a string, and allows
  userstate  which is an integer.
  I wonder if we can convince PKI to use a different schema to reprsent
  this information. We can use Roles or Groups to tell what type of user a
  user is, not sure about the state as that schema file has exactly the
  same comment for both usertype and userstate, seems a bug.
 
 I think so.  I do not know if CMSuser is really needed, as it looks like 
 everything in there is accounted for some other way.
 
 My guess is the type  field is used to enforce that someone in one 
 group,  say agents, cannot also be in a different group, say auditors 
 but they do use groups for  most roles in the system.
 
 I think that there are going to be severl layers for the permissions in 
 the IPA approach:  For CAAdmins, we create a group CAdmins, and  add a 
 Role to to CAAdminRole which contains the appropriate Permissions.  Same 
 for Auditors and agents.  No one should have the ACI to change these roles.
 
 We probably want to put in an RFE for IPA Roles  that state two roles 
 are mutually exclusive.
 
 
 userstate is, I think, to disable an account, which is handled using 
 nsaccount lock  in IPA.  I think we should agree on a single mechanism, 
 and it should be the one in the most standard schema.
 
 

With just an initial look at the dogtag code, it appears that userState
and userType are no longer used in any meaningful way.  We'll have to do
a more exhaustive search to be sure, but initial indications are that we
may no longer need this object class.  

Adam does bring up a good point, which is that - as a common criteria
requirement, users were required to have no more than one of the
following roles: agent, admin, auditor.  How would this be enforced in
the IPA database?
  
 
 
 
  IIRC the user we create in CS now has the description attribute set up
  in a very specific way. Is that still required?
  What is description used for ?
 
  Simo.
 
 
 
 ___
 Freeipa-devel mailing list
 Freeipa-devel@redhat.com
 https://www.redhat.com/mailman/listinfo/freeipa-devel


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

Re: [Freeipa-devel] Unifying the PKI and IPA Directory Server instances

2011-11-03 Thread Adam Young


On 11/03/2011 11:00 AM, Ade Lee wrote:

On Thu, 2011-11-03 at 09:20 -0400, Adam Young wrote:

On 11/03/2011 12:56 AM, Simo Sorce wrote:

On Wed, 2011-11-02 at 20:25 -0400, Adam Young wrote:

On 11/02/2011 06:19 PM, Rob Crittenden wrote:

Simo Sorce wrote:

On Wed, 2011-11-02 at 16:44 -0400, Ade Lee wrote:

On Wed, 2011-11-02 at 16:03 -0400, Adam Young wrote:

[...]

So, a user becomes an agent on the ca by having a certificate in the
user record and being a member of the relevant admin, agent or auditor
group.

I see this as follows:
1. ipa cms-user-add (add a user and add the auxilliary cmsuser object
class)
2. ipa user-cert (contact the ca and get a certificate for this user,
add this cert to the user record in the ipa database)
3. ipa group-add-member (add the user to the relevant group)

At no point does PKI need to modify anything in the IPA database.

Sounds reasonable.
Can you post a link to the schema that would be added to IPA objects ?

Simo.


I think this is it:

http://svn.fedorahosted.org/svn/pki/trunk/pki/base/ca/shared/conf/schema.ldif

Look for cmsuser.

Unfortunately it looks like the cmsuser objectclass is of type
structural, which means it cannot be added to existing records.


The cert seems to  comes from

05rfc4523.ldif

and is added in

06inetorgperson.ldif

Which is already in our user record.

CMS only seems to require usertype, which is a string, and allows
userstate  which is an integer.

I wonder if we can convince PKI to use a different schema to reprsent
this information. We can use Roles or Groups to tell what type of user a
user is, not sure about the state as that schema file has exactly the
same comment for both usertype and userstate, seems a bug.

I think so.  I do not know if CMSuser is really needed, as it looks like
everything in there is accounted for some other way.

My guess is the type  field is used to enforce that someone in one
group,  say agents, cannot also be in a different group, say auditors
but they do use groups for  most roles in the system.

I think that there are going to be severl layers for the permissions in
the IPA approach:  For CAAdmins, we create a group CAdmins, and  add a
Role to to CAAdminRole which contains the appropriate Permissions.  Same
for Auditors and agents.  No one should have the ACI to change these roles.

We probably want to put in an RFE for IPA Roles  that state two roles
are mutually exclusive.


userstate is, I think, to disable an account, which is handled using
nsaccount lock  in IPA.  I think we should agree on a single mechanism,
and it should be the one in the most standard schema.



With just an initial look at the dogtag code, it appears that userState
and userType are no longer used in any meaningful way.  We'll have to do
a more exhaustive search to be sure, but initial indications are that we
may no longer need this object class.

Adam does bring up a good point, which is that - as a common criteria
requirement, users were required to have no more than one of the
following roles: agent, admin, auditor.  How would this be enforced in
the IPA database?


I think that we need to make it an RFE for IPA.









IIRC the user we create in CS now has the description attribute set up
in a very specific way. Is that still required?

What is description used for ?

Simo.



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




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

Re: [Freeipa-devel] Unifying the PKI and IPA Directory Server instances

2011-11-03 Thread Simo Sorce

On Thu, 2011-11-03 at 11:00 -0400, Ade Lee wrote:
 On Thu, 2011-11-03 at 09:20 -0400, Adam Young wrote:
  On 11/03/2011 12:56 AM, Simo Sorce wrote:
   On Wed, 2011-11-02 at 20:25 -0400, Adam Young wrote:
   On 11/02/2011 06:19 PM, Rob Crittenden wrote:
   Simo Sorce wrote:
   On Wed, 2011-11-02 at 16:44 -0400, Ade Lee wrote:
   On Wed, 2011-11-02 at 16:03 -0400, Adam Young wrote:
   [...]
   So, a user becomes an agent on the ca by having a certificate in the
   user record and being a member of the relevant admin, agent or auditor
   group.
  
   I see this as follows:
   1. ipa cms-user-add (add a user and add the auxilliary cmsuser object
   class)
   2. ipa user-cert (contact the ca and get a certificate for this user,
   add this cert to the user record in the ipa database)
   3. ipa group-add-member (add the user to the relevant group)
  
   At no point does PKI need to modify anything in the IPA database.
   Sounds reasonable.
   Can you post a link to the schema that would be added to IPA objects ?
  
   Simo.
  
   I think this is it:
  
   http://svn.fedorahosted.org/svn/pki/trunk/pki/base/ca/shared/conf/schema.ldif
  
   Look for cmsuser.
   Unfortunately it looks like the cmsuser objectclass is of type
   structural, which means it cannot be added to existing records.
  
   The cert seems to  comes from
  
   05rfc4523.ldif
  
   and is added in
  
   06inetorgperson.ldif
  
   Which is already in our user record.
  
   CMS only seems to require usertype, which is a string, and allows
   userstate  which is an integer.
   I wonder if we can convince PKI to use a different schema to reprsent
   this information. We can use Roles or Groups to tell what type of user a
   user is, not sure about the state as that schema file has exactly the
   same comment for both usertype and userstate, seems a bug.
  
  I think so.  I do not know if CMSuser is really needed, as it looks like 
  everything in there is accounted for some other way.
  
  My guess is the type  field is used to enforce that someone in one 
  group,  say agents, cannot also be in a different group, say auditors 
  but they do use groups for  most roles in the system.
  
  I think that there are going to be severl layers for the permissions in 
  the IPA approach:  For CAAdmins, we create a group CAdmins, and  add a 
  Role to to CAAdminRole which contains the appropriate Permissions.  Same 
  for Auditors and agents.  No one should have the ACI to change these roles.
  
  We probably want to put in an RFE for IPA Roles  that state two roles 
  are mutually exclusive.
  
  
  userstate is, I think, to disable an account, which is handled using 
  nsaccount lock  in IPA.  I think we should agree on a single mechanism, 
  and it should be the one in the most standard schema.
  
  
 
 With just an initial look at the dogtag code, it appears that userState
 and userType are no longer used in any meaningful way.  We'll have to do
 a more exhaustive search to be sure, but initial indications are that we
 may no longer need this object class.  
 
 Adam does bring up a good point, which is that - as a common criteria
 requirement, users were required to have no more than one of the
 following roles: agent, admin, auditor.  How would this be enforced in
 the IPA database?

At the moment it can't be enforced, but I guess we could build a special
plugin that will work similarly to the uniqueness plugin but will work
on one or more pools of mutually exclusive roles to be enforced on all
entries. I guess this is something that could be useful outside of CA as
well if roles turns out to be used more.

Simo.

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

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

Re: [Freeipa-devel] Unifying the PKI and IPA Directory Server instances

2011-11-03 Thread Ade Lee

On Thu, 2011-11-03 at 09:22 -0400, Rob Crittenden wrote:
 Ade Lee wrote:
  On Wed, 2011-11-02 at 16:03 -0400, Adam Young wrote:
  To clarify:  there are two types of Data stored in the PKI CA DS
  instances.  One is Users and groups (IdM),  and the other is
  certificates and requests.
 
  The CA currently administers its own users:  creates, add deletes, add
  privs and so forth.  If  we extract the IdM  objects from the CA
  control, we will need to build another mechanism to manage them.
 
  The Certificates will stay in their own suffix.  I don't think anyone
  disagrees about this.
 
  I'm trying to think through the steps of using the IPA user database for
  PKI.  If I undertand the end state, the general flow will be driven from
  ipa-server-install and will look like this:
 
  1.  Create a unified DS instance.  We can continue to name it the way
  that IPA does:   All caps the hostname.
  2.  Apply the LDAP schema LDIFs to it.  At this point, we probably want
  to create the whole IPA schema,  and the cmsUser as well
 
  For clarification, cmsuser is just an object that is defined in the PKI
  schema, not the actual admin user created in the install itself.
 
  It may be advantageous to actually pre-create this user when applying
  schema LDIFS and just have pkisilent add the user certs.
 
 The description attribute needs to store per-instance specific 
 information such as the requestId. Unless you mean just adding userstate 
 and usertype.
 

In dogtag, I believe we have used the description attribute to store
whatever the user provided to describe the user/group.  This is more
relevant to the console.

As IPA will be managing users and groups, then it can also manage this
attribute.

  3.  Call PKISilent (or equivalent) passing the info for  the Unified
  directory server instance, to include the IPA tree for the users.
  4.  PKISilent will create the PKI admin user,  to include its
  certificiate in the IPA tree.  This user will be half baked from an IPA
  perspective,  but will allow administration of the CA.
 
  Pre-creating this user may solve this problem.  You can pre-create a
  user with all the required IPA and PKI attributes and just have
  pkisilent add the user cert needed.
 
  As we will be running as directory manager in this case, we will
  permissions to do this.
 
  5.  Create a PKIDirectory Manager account that has complete control over
  only the PKI  suffix, and not the IPA suffix.  Modify the PKI code to
  use only this account.  This account should be able to authenticate
  against the Dir Srv using a client certificate, stored in the PKI
  specific NSS database.
 
  This of course involves setting up the relevant ACIs.  We may want to
  consider the reverse.  That only certain IPA accounts should have access
  to the PKI data.
 
 Which still involves ACIs, right?
Right
 
  6.  Restart the CA.
  7.  Continue the IPA server install from the.  Fix up the Admin user so
  that it works for IPA.
 
  Not needed if we pre-populate as mentioned above.
 
  8.  Disable the Directory Manager's password.  From here on out,  access
  to the Dir Srv should be either certificate or Keytab only.
 
 
  After IPA is up and running, the management of the User Database is done
  by IPA.
  One thing that several people have asked for in IPA is the ability to
  manage external Roles:  ones that don't map to ACIs.  PKI has this
  requirement as well.  There are a couple approaches we could take:
 
  1.  Attempt to use the current Role mechanism in IPA.  This gets hidden
  to an outside user,  but that should be OK.  Checking if a user is a PKI
  Admin, Agent, or Auditor should be done only by a privileged account 
  anyway.
 
  2.  Use a User Group:  PKIAdmins,  PKIAgents,  and PKIAuditors.This
  is what I would suggest.
 
  3.  Create an external mechanism.
 
 
  Once IPA has assumed control of the IdM DB, we will still need to be
  able to get user certificates into the  user records CMSUser field.  I
  do not know CS well enough to say how that would work,  but I assume it
  will be one of these two mechanisms:
 
  1.  Use the CA Administrative interface to modify an existing user to
  have the certificate
  or
  2.  Add a mechanism to IPA to request and apply the certificate to the
  IPA User.
 
  I'm guessing that the flow would be something like this:
 
  1.  Create a user  (ipa user-add)
  2.  Assign them to the PKIAgents groups
  3.  Call the CA Admin interface to make the user an agent.
 
  If we do it this way, the  PKI instance will need to be able to modify
  the IPA user record.  Alternatively:
 
  1.  ipa user-add
  2.  ipa group-add-member
  3.  ipa user-cert
 
  So, a user becomes an agent on the ca by having a certificate in the
  user record and being a member of the relevant admin, agent or auditor
  group.
 
  I see this as follows:
  1. ipa cms-user-add (add a user and add the auxilliary cmsuser object
  class)
  2. ipa user-cert (contact the ca and get a certificate for

Re: [Freeipa-devel] Unifying the PKI and IPA Directory Server instances

2011-11-03 Thread Andrew Wnuk


On 11/02/2011 03:19 PM, Rob Crittenden wrote:

Simo Sorce wrote:

On Wed, 2011-11-02 at 16:44 -0400, Ade Lee wrote:

On Wed, 2011-11-02 at 16:03 -0400, Adam Young wrote:

[...]

So, a user becomes an agent on the ca by having a certificate in the
user record and being a member of the relevant admin, agent or auditor
group.

I see this as follows:
1. ipa cms-user-add (add a user and add the auxilliary cmsuser object
class)
2. ipa user-cert (contact the ca and get a certificate for this user,
add this cert to the user record in the ipa database)
3. ipa group-add-member (add the user to the relevant group)

At no point does PKI need to modify anything in the IPA database.


Sounds reasonable.
Can you post a link to the schema that would be added to IPA objects ?

Simo.



IIRC the user we create in CS now has the description attribute set up 
in a very specific way. Is that still required?


rob


Steps 1 to 3 should have an option to be performed only by CS admins 
with certificate client authentication, otherwise we will break rules of 
secure CS configuration including separation of roles.


Andrew

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

Re: [Freeipa-devel] Unifying the PKI and IPA Directory Server instances

2011-11-03 Thread Adam Young


On 11/03/2011 11:30 AM, Andrew Wnuk wrote:

On 11/02/2011 03:19 PM, Rob Crittenden wrote:

Simo Sorce wrote:

On Wed, 2011-11-02 at 16:44 -0400, Ade Lee wrote:

On Wed, 2011-11-02 at 16:03 -0400, Adam Young wrote:

[...]

So, a user becomes an agent on the ca by having a certificate in the
user record and being a member of the relevant admin, agent or auditor
group.

I see this as follows:
1. ipa cms-user-add (add a user and add the auxilliary cmsuser object
class)
2. ipa user-cert (contact the ca and get a certificate for this user,
add this cert to the user record in the ipa database)
3. ipa group-add-member (add the user to the relevant group)

At no point does PKI need to modify anything in the IPA database.


Sounds reasonable.
Can you post a link to the schema that would be added to IPA objects ?

Simo.



IIRC the user we create in CS now has the description attribute set 
up in a very specific way. Is that still required?


rob


Steps 1 to 3 should have an option to be performed only by CS admins 
with certificate client authentication, otherwise we will break rules 
of secure CS configuration including separation of roles.



We had a long talk about that on the IPA call this morning.

In order to add someone to the PKIAdmin  user-group,  you need to have 
the appropriate ACIs.  We'd like to lock thos in,  so that someone 
messing around with IPA can't mess them up.


I'm not certain that the specific authentication mechanism  is the issue 
so much as  you need to have a guarantee of  authentication no less than 
what Client Cert auth gives you.  Kerberos authentication should  
actually be as good:  it will be enforced not just by the application, 
but all the way down to the DS instance  via ACIs.




Andrew

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


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

[Freeipa-devel] [PATCHES] #2036 Fix coverity bugs

2011-11-03 Thread Simo Sorce

Just some unchecked returns, we do not care much, but will keep Coverity
happy.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York
From b82d9e3b04de4ad8addba68dcc06d9155f1b613d Mon Sep 17 00:00:00 2001
From: Simo Sorce sso...@redhat.com
Date: Thu, 3 Nov 2011 10:06:48 -0400
Subject: [PATCH 1/3] Fix CID 10742: Unchecked return value

https://fedorahosted.org/freeipa/ticket/2036
---
 .../ipa-enrollment/ipa_enrollment.c|2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/daemons/ipa-slapi-plugins/ipa-enrollment/ipa_enrollment.c b/daemons/ipa-slapi-plugins/ipa-enrollment/ipa_enrollment.c
index 1f5ce9b477a6152e3ef7befeea1230ab75dfba70..9f884bd39233adf90b0f4eff1868885d587d351a 100644
--- a/daemons/ipa-slapi-plugins/ipa-enrollment/ipa_enrollment.c
+++ b/daemons/ipa-slapi-plugins/ipa-enrollment/ipa_enrollment.c
@@ -451,7 +451,7 @@ ipaenrollment_init(Slapi_PBlock *pb)
 if (!ret) ret = slapi_pblock_set(pb, SLAPI_PLUGIN_DESCRIPTION, (void *)pdesc);
 if (!ret) ret = slapi_pblock_set(pb, SLAPI_PLUGIN_EXT_OP_OIDLIST, ipaenrollment_oid_list);
 if (!ret) ret = slapi_pblock_set(pb, SLAPI_PLUGIN_EXT_OP_NAMELIST, ipaenrollment_name_list);
-if (!ret) slapi_pblock_set(pb, SLAPI_PLUGIN_EXT_OP_FN, (void *)ipaenrollment_extop);
+if (!ret) ret = slapi_pblock_set(pb, SLAPI_PLUGIN_EXT_OP_FN, (void *)ipaenrollment_extop);
 
 if (ret) {
 LOG(Failed to set plug-in version, function, and OID.\n);
-- 
1.7.6.4

From 7cc06308f02247d5076ae85fff22eb66cfdc1556 Mon Sep 17 00:00:00 2001
From: Simo Sorce sso...@redhat.com
Date: Thu, 3 Nov 2011 10:05:21 -0400
Subject: [PATCH 2/3] Fix CID 10743: Unchecked return value

https://fedorahosted.org/freeipa/ticket/2036
---
 .../ipa-pwd-extop/ipa_pwd_extop.c  |   10 --
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
index 95ac68e9cfc8d7024048d8a9d2793044f01dd1aa..a0f9c5e14747b5c38952db27b005874a4afe1c4d 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
@@ -504,9 +504,15 @@ free_and_return:
 	/* Either this is the same pointer that we allocated and set above,
 	 * or whoever used it should have freed it and allocated a new
 	 * value that we need to free here */
-	slapi_pblock_get(pb, SLAPI_ORIGINAL_TARGET, dn);
+ret = slapi_pblock_get(pb, SLAPI_ORIGINAL_TARGET, dn);
+if (ret) {
+LOG_TRACE(Failed to get SLAPI_ORIGINAL_TARGET\n);
+}
 	slapi_ch_free_string(dn);
-	slapi_pblock_set(pb, SLAPI_ORIGINAL_TARGET, NULL);
+ret = slapi_pblock_set(pb, SLAPI_ORIGINAL_TARGET, NULL);
+if (ret) {
+LOG_TRACE(Failed to clear SLAPI_ORIGINAL_TARGET\n);
+}
 	slapi_ch_free_string(authmethod);
 slapi_ch_free_string(principal);
 
-- 
1.7.6.4

From b8d3ef5b3f09193214fb8bf7220a71775895ce1e Mon Sep 17 00:00:00 2001
From: Simo Sorce sso...@redhat.com
Date: Thu, 3 Nov 2011 09:57:41 -0400
Subject: [PATCH 3/3] Fix CID 10745: Unchecked return value

https://fedorahosted.org/freeipa/ticket/2036
---
 .../ipa-pwd-extop/ipa_pwd_extop.c  |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
index a0f9c5e14747b5c38952db27b005874a4afe1c4d..65c5834595f89aee8502347311f247be058c3416 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
@@ -1295,7 +1295,7 @@ int ipapwd_init( Slapi_PBlock *pb )
 if (!ret) ret = slapi_pblock_set(pb, SLAPI_PLUGIN_DESCRIPTION, (void *)ipapwd_plugin_desc);
 if (!ret) ret = slapi_pblock_set(pb, SLAPI_PLUGIN_EXT_OP_OIDLIST, ipapwd_oid_list);
 if (!ret) ret = slapi_pblock_set(pb, SLAPI_PLUGIN_EXT_OP_NAMELIST, ipapwd_name_list);
-if (!ret) slapi_pblock_set(pb, SLAPI_PLUGIN_EXT_OP_FN, (void *)ipapwd_extop);
+if (!ret) ret = slapi_pblock_set(pb, SLAPI_PLUGIN_EXT_OP_FN, (void *)ipapwd_extop);
 if (ret) {
 LOG(Failed to set plug-in version, function, and OID.\n );
 return -1;
-- 
1.7.6.4

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

[Freeipa-devel] [PATCHES] #2037 Coverity issues

2011-11-03 Thread Simo Sorce

These are actual issues. Most are resource leaks.
And one is a bad sizeof() computation that will cause us later to
overwrite out of bound memory, so potentially a segfault.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York
From dff903a1f3a15516c36f40d4dffeaf751dba6423 Mon Sep 17 00:00:00 2001
From: Simo Sorce sso...@redhat.com
Date: Thu, 3 Nov 2011 13:21:59 -0400
Subject: [PATCH 1/9] Fix CID 11019: Resource leak

https://fedorahosted.org/freeipa/ticket/2037
---
 daemons/ipa-kdb/ipa_kdb.c |   13 +++--
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/daemons/ipa-kdb/ipa_kdb.c b/daemons/ipa-kdb/ipa_kdb.c
index 6a6c2063902f8b2a76d97f3510f09333c5af168d..481b1f392766498c5d7c6333fe73bafefde87dae 100644
--- a/daemons/ipa-kdb/ipa_kdb.c
+++ b/daemons/ipa-kdb/ipa_kdb.c
@@ -263,6 +263,13 @@ int ipadb_get_connection(struct ipadb_context *ipactx)
 
 done:
 ldap_msgfree(res);
+
+ldap_value_free_len(vals);
+for (i = 0; i  c  cvals[i]; i++) {
+free(cvals[i]);
+}
+free(cvals);
+
 if (ret) {
 if (ipactx-lcontext) {
 ldap_unbind_ext_s(ipactx-lcontext, NULL, NULL);
@@ -274,12 +281,6 @@ done:
 return EIO;
 }
 
-ldap_value_free_len(vals);
-for (i = 0; i  c; i++) {
-free(cvals[i]);
-}
-free(cvals);
-
 return 0;
 }
 
-- 
1.7.6.4

From d83e698e2f998767cec8c2505dff666f9c51 Mon Sep 17 00:00:00 2001
From: Simo Sorce sso...@redhat.com
Date: Thu, 3 Nov 2011 13:26:45 -0400
Subject: [PATCH 2/9] Fix CID 11020: Resource leak

https://fedorahosted.org/freeipa/ticket/2037
---
 daemons/ipa-kdb/ipa_kdb_passwords.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/daemons/ipa-kdb/ipa_kdb_passwords.c b/daemons/ipa-kdb/ipa_kdb_passwords.c
index 93e9e206081af412a472ab0c7624611a628a15b7..0bb7fa72496789241e27ecc852a1c6ede7f8e40a 100644
--- a/daemons/ipa-kdb/ipa_kdb_passwords.c
+++ b/daemons/ipa-kdb/ipa_kdb_passwords.c
@@ -203,6 +203,7 @@ krb5_error_code ipadb_change_pwd(krb5_context context,
 ret = asprintf(ied-pw_policy_dn,
cn=global_policy,%s, ipactx-realm_base);
 if (ret == -1) {
+free(ied);
 return ENOMEM;
 }
 db_entry-e_data = (krb5_octet *)ied;
-- 
1.7.6.4

From bd68245ff2ea1a1a039dda19bb04c308500ac57d Mon Sep 17 00:00:00 2001
From: Simo Sorce sso...@redhat.com
Date: Thu, 3 Nov 2011 13:40:46 -0400
Subject: [PATCH 3/9] Fix CID 11021: Resource leak

https://fedorahosted.org/freeipa/ticket/2037
---
 util/ipa_pwd.c |4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/util/ipa_pwd.c b/util/ipa_pwd.c
index c41617533ec25e8e656e9bb7a69d5b8b5dd8b5f7..fda6cb34ef24059362207325db61aedb62d7b665 100644
--- a/util/ipa_pwd.c
+++ b/util/ipa_pwd.c
@@ -560,7 +560,7 @@ int ipapwd_generate_new_history(char *password,
 unsigned char *hash = NULL;
 unsigned int hash_len;
 char *new_element;
-char **ordered;
+char **ordered = NULL;
 int c, i, n;
 int len;
 int ret;
@@ -626,9 +626,11 @@ int ipapwd_generate_new_history(char *password,
 
 *new_pwd_history = ordered;
 *new_pwd_hlen = n;
+ordered = NULL;
 ret = IPAPWD_POLICY_OK;
 
 done:
+free(ordered);
 free(hash);
 return ret;
 }
-- 
1.7.6.4

From 44c5f0588cbce8ee38b14e17fe0a07fe33cdfb0f Mon Sep 17 00:00:00 2001
From: Simo Sorce sso...@redhat.com
Date: Thu, 3 Nov 2011 13:45:06 -0400
Subject: [PATCH 4/9] Fix CID 11022: Resource leak

https://fedorahosted.org/freeipa/ticket/2037
---
 daemons/ipa-kdb/ipa_kdb_principals.c |7 +++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c b/daemons/ipa-kdb/ipa_kdb_principals.c
index fdd834f355fd9e056058fa205b217e9e1f142e51..117eea86952ee4662930b80ba5e54c75aa587faf 100644
--- a/daemons/ipa-kdb/ipa_kdb_principals.c
+++ b/daemons/ipa-kdb/ipa_kdb_principals.c
@@ -1571,6 +1571,7 @@ static krb5_error_code ipadb_entry_to_mods(krb5_context kcontext,
 char **new_history;
 int nh_len;
 int ret;
+int i;
 
 ied = (struct ipadb_e_data *)entry-e_data;
 if (ied-magic != IPA_E_DATA_MAGIC) {
@@ -1619,6 +1620,12 @@ static krb5_error_code ipadb_entry_to_mods(krb5_context kcontext,
 
 kerr = ipadb_get_ldap_mod_str_list(imods, passwordHistory,
new_history, nh_len, mod_op);
+
+for (i = 0; i  nh_len; i++) {
+free(new_history[i]);
+}
+free(new_history);
+
 if (kerr) {
 goto done;
 }
-- 
1.7.6.4

From 5faf3e64d6d418be9daad1a2bb600817f29088dc Mon Sep 17 00:00:00 2001
From: Simo Sorce sso...@redhat.com
Date: Thu, 3 Nov 2011 13:48:32 -0400
Subject: [PATCH 5/9] Fix CID 11023: Resource leak

https://fedorahosted.org/freeipa/ticket/2037
---
 daemons/ipa-kdb/ipa_kdb_principals.c |1 +
 1 files changed, 1 insertions(+), 0

Re: [Freeipa-devel] [PATCH] 158 Fix ipa-server-install answer cache

2011-11-03 Thread Rob Crittenden


Martin Kosek wrote:

Current Answer Cache storing mechanism is not ideal for storing
non-trivial Python types like arrays, custom classes, etc.
RawConfigParser just translates values to string, which
are not correctly decoded when the Answer Cache is parsed and
restored in the installer.

This patch replaces RawConfigParser with Python's standard pickle
module, which is a recommended way for serialization in Python.

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


ack

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

Re: [Freeipa-devel] [PATCH] 029 Page is cleared before it is visible

2011-11-03 Thread Endi Sukma Dewata


On 11/2/2011 11:01 AM, Petr Vobornik wrote:

Regardless, ACK and pushed to master.


Found another problem, the krbtpolicy  config need to be forced to 
update. See the attached patch.


--
Endi S. Dewata
From fc3895f69dbf15f37d81f5dffad59a17a2953a03 Mon Sep 17 00:00:00 2001
From: Endi S. Dewata edew...@redhat.com
Date: Thu, 3 Nov 2011 16:02:32 -0500
Subject: [PATCH] Fixed blank krbtpolicy and config pages.

The details page compares the old and the new primary keys to determine
if the page needs to be reloaded. The Kerberos Ticket Policy and Config
pages do not use primary keys, so they are never loaded/updated with
data. A parameter has been added to force update on these pages.

Ticket #1459
---
 install/ui/association.js  |1 +
 install/ui/details.js  |1 +
 install/ui/entity.js   |2 ++
 install/ui/policy.js   |   15 +++
 install/ui/serverconfig.js |   13 +++--
 5 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/install/ui/association.js b/install/ui/association.js
index d3d6b124b431414ff04fad05b16dbb972b38c2b7..6ce8fea46caa57638273d53518ce0472df58ac2d 100644
--- a/install/ui/association.js
+++ b/install/ui/association.js
@@ -1206,6 +1206,7 @@ IPA.association_facet = function (spec) {
 };
 
 that.needs_update = function() {
+if (that._needs_update !== undefined) return that._needs_update;
 var pkey = IPA.nav.get_state(that.entity.name+'-pkey');
 return that.pkey !== pkey;
 };
diff --git a/install/ui/details.js b/install/ui/details.js
index 15056204f72ef2095862c2c35d24cd47fbc819b3..93bb3e8a404bb24374e64bfeb824869f531a7f96 100644
--- a/install/ui/details.js
+++ b/install/ui/details.js
@@ -528,6 +528,7 @@ IPA.details_facet = function(spec) {
 };
 
 that.needs_update = function() {
+if (that._needs_update !== undefined) return that._needs_update;
 var pkey = IPA.nav.get_state(that.entity.name+'-pkey');
 return pkey !== that.pkey;
 };
diff --git a/install/ui/entity.js b/install/ui/entity.js
index ace44c3c1fef43e8a8700038c5305391ae3106a4..526e2795d94512add214af785a8442e18192d171 100644
--- a/install/ui/entity.js
+++ b/install/ui/entity.js
@@ -44,6 +44,7 @@ IPA.facet = function (spec) {
 that.header = spec.header || IPA.facet_header({ facet: that });
 
 that.entity_name = spec.entity_name;
+that._needs_update = spec.needs_update;
 
 that.dialogs = $.ordered_map();
 
@@ -113,6 +114,7 @@ IPA.facet = function (spec) {
 };
 
 that.needs_update = function() {
+if (that._needs_update !== undefined) return that._needs_update;
 return true;
 };
 
diff --git a/install/ui/policy.js b/install/ui/policy.js
index 41432f743e2b894cb4a8d2a9b338bacc85b8c762..f773c4714bf1f62305d261dd42a8bc20195f8d9b 100644
--- a/install/ui/policy.js
+++ b/install/ui/policy.js
@@ -76,9 +76,16 @@ IPA.entity_factories.krbtpolicy =  function() {
 entity('krbtpolicy').
 details_facet({
 title: IPA.metadata.objects.krbtpolicy.label,
-sections:[{
-name: 'identity',
-fields:[ 'krbmaxrenewableage','krbmaxticketlife' ]
-}]}).
+sections: [
+{
+name: 'identity',
+fields: [
+'krbmaxrenewableage',
+'krbmaxticketlife'
+]
+}
+],
+needs_update: true
+}).
 build();
 };
diff --git a/install/ui/serverconfig.js b/install/ui/serverconfig.js
index 6dc2eaf6dec7ae49a69fe91ecb7779076534ab62..eec47d2f77647427f5a465f2bec1dd8648a8b506 100644
--- a/install/ui/serverconfig.js
+++ b/install/ui/serverconfig.js
@@ -32,12 +32,11 @@ IPA.entity_factories.config = function(){
 entity('config').
 details_facet({
 title: IPA.metadata.objects.config.label,
-sections:
-[
+sections: [
 {
 name: 'search',
 label: IPA.messages.objects.config.search,
-fields:[
+fields: [
 'ipasearchrecordslimit',
 'ipasearchtimelimit'
 ]
@@ -45,7 +44,7 @@ IPA.entity_factories.config = function(){
 {
 name: 'user',
 label: IPA.messages.objects.config.user,
-fields:[
+fields: [
 'ipausersearchfields',
 'ipadefaultemaildomain',
 {
@@ -71,7 +70,7 @@ IPA.entity_factories.config = function(){
 {
 name: 'group',
 label: IPA.messages.objects.config.group,
-fields:[
+fields: [
 'ipagroupsearchfields',
 {
 factory:

Re: [Freeipa-devel] [PATCH] 158 Fix ipa-server-install answer cache

2011-11-03 Thread Martin Kosek

On Thu, 2011-11-03 at 15:02 -0400, Rob Crittenden wrote:
 Martin Kosek wrote:
  Current Answer Cache storing mechanism is not ideal for storing
  non-trivial Python types like arrays, custom classes, etc.
  RawConfigParser just translates values to string, which
  are not correctly decoded when the Answer Cache is parsed and
  restored in the installer.
 
  This patch replaces RawConfigParser with Python's standard pickle
  module, which is a recommended way for serialization in Python.
 
  https://fedorahosted.org/freeipa/ticket/2054
 
 ack
 

Pushed to master, ipa-2-1.

Martin

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

[Freeipa-devel] [PATCH] #2038 modify salt creation

2011-11-03 Thread Simo Sorce

As stated in the bug in order to attain better interoperability with
Windows clients we need to change the way we generate the random salt.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York
From 350fdbfeab1cd04ba1ef576f7de075dbb91b6dfc Mon Sep 17 00:00:00 2001
From: Simo Sorce sso...@redhat.com
Date: Thu, 3 Nov 2011 16:15:10 -0400
Subject: [PATCH] Modify random salt creation for interoperability

See:
https://fedorahosted.org/freeipa/ticket/2038
---
 util/ipa_krb5.c |   37 +
 1 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/util/ipa_krb5.c b/util/ipa_krb5.c
index 5b6fc5821a04614030353a376f33d2ca89bc86b2..ba9d3cefce0944d790715c3249f158b9f0ae232d 100644
--- a/util/ipa_krb5.c
+++ b/util/ipa_krb5.c
@@ -9,6 +9,34 @@
 /* Salt types */
 #define KRB5P_SALT_SIZE 16
 
+static krb5_error_code ipa_get_random_salt(krb5_context krbctx,
+   krb5_data *salt)
+{
+krb5_error_code kerr;
+int i;
+
+/* make random salt */
+salt-length = KRB5P_SALT_SIZE;
+salt-data = malloc(KRB5P_SALT_SIZE);
+if (!salt-data) {
+return ENOMEM;
+}
+kerr = krb5_c_random_make_octets(krbctx, salt);
+if (kerr) {
+return kerr;
+}
+
+/* Windows treats the salt as a string.
+ * To avoid any compatibility issue, limits octects only to
+ * the ASCII printable range, or 0x20 = val = 0x7E */
+for (i = 0; i  salt-length; i++) {
+salt-data[i] %= 0x5E; /* 7E - 20 */
+salt-data[i] += 0x20; /* add base */
+}
+
+return 0;
+}
+
 void
 ipa_krb5_free_ktypes(krb5_context context, krb5_enctype *val)
 {
@@ -125,14 +153,7 @@ krb5_error_code ipa_krb5_generate_key_data(krb5_context krbctx,
 
 case KRB5_KDB_SALTTYPE_SPECIAL:
 
-/* make random salt */
-salt.length = KRB5P_SALT_SIZE;
-salt.data = malloc(KRB5P_SALT_SIZE);
-if (!salt.data) {
-kerr = ENOMEM;
-goto done;
-}
-kerr = krb5_c_random_make_octets(krbctx, salt);
+kerr = ipa_get_random_salt(krbctx, salt);
 if (kerr) {
 goto done;
 }
-- 
1.7.7

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

Re: [Freeipa-devel] [PATCH] 030 Extending facet's mechanism of gathering changes

2011-11-03 Thread Endi Sukma Dewata


On 11/3/2011 7:35 AM, Petr Vobornik wrote:

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

I'm not sure if update_info and other new classes should be in details.js.


It's probably ok now, but in the future we might want to move them 
somewhere else. How about creating command.js and move all 
command-related code there?


Other issues:

1. In details.js:650 we don't use param_info anymore, it should be metadata.

2. The add_field_option(command, field_name, param_info, values, join) 
probably can be simplified into add_field_option(field, values). The 
IPA.command_builder can store the command internally:


  var command_builder = IPA.command_builder({
  command: command
  });

  ...

  command_builder.add_field_option(field_info.field, values);

The add_field_option() can get the name, metadata, and join from the 
field object.


3. The add_record_to_command() takes a list of sections, but it might 
not be necessary because (so far) it's only used to access the details 
facet's own list of sections. Do you expect this to be used differently?


4. The create_fields_update_command() is essentially the same as 
create_standard_update_command(). When the command_mode is 'save' is it 
possible to generate an update_info from records so we can just call 
create_fields_update_command()?


This patch I think can be ACKed after fixing #1, the rest can be dealt 
with later.


Some of the new codes are not used anywhere yet so it's a bit difficult 
to review and things might change again later. I'm curious to see how 
it's going to be used in HBAC/sudo rule and DNS zone that involves 
multiple commands: how the commands are going to be defined, how to 
associate certain fields with certain commands, how to assign 
priorities, etc. Do you have any preview patch for ticket #1515?


--
Endi S. Dewata

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

Re: [Freeipa-devel] [PATCH] 031 Field for DNS SOA class changed to combobox with options

2011-11-03 Thread Endi Sukma Dewata


On 11/3/2011 9:26 AM, Petr Vobornik wrote:

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


ACK and pushed to master.

--
Endi S. Dewata

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

[Freeipa-devel] [PATCH] 307 Added extensible UI framework.

2011-11-03 Thread Endi Sukma Dewata


The entity definitions have been converted into classes. The entity
init() method will use the builder to construct the facets and dialogs.
The UI can be customized by creating a subclass of the original entity
in extension.js and then overriding the init() method.

Ticket #2043

--
Endi S. Dewata
From 509cf7f568d763058f6133a6f71feb5ccb5c4b97 Mon Sep 17 00:00:00 2001
From: Endi S. Dewata edew...@redhat.com
Date: Wed, 2 Nov 2011 14:07:07 -0500
Subject: [PATCH] Added extensible UI framework.

The entity definitions have been converted into classes. The entity
init() method will use the builder to construct the facets and dialogs.
The UI can be customized by creating a subclass of the original entity
in extension.js and then overriding the init() method.

Ticket #2043
---
 install/ui/aci.js  |   87 +---
 install/ui/automount.js|   56 +++--
 install/ui/dns.js  |   38 +---
 install/ui/entitle.js  |   31 ++--
 install/ui/entity.js   |   10 +++---
 install/ui/group.js|   22 
 install/ui/hbac.js |   56 +++-
 install/ui/host.js |   20 +++---
 install/ui/hostgroup.js|   20 ++
 install/ui/index.html  |2 +-
 install/ui/ipa.js  |   29 +++
 install/ui/netgroup.js |   20 +++---
 install/ui/policy.js   |   46 +++-
 install/ui/serverconfig.js |   21 +++
 install/ui/service.js  |   21 +++
 install/ui/sudo.js |   55 ++--
 install/ui/user.js |   19 ++
 17 files changed, 355 insertions(+), 198 deletions(-)

diff --git a/install/ui/aci.js b/install/ui/aci.js
index 5ffb2108b51b2e30113221d9ab1af107635eb8b4..92c5dcf02db36dfe48be4b80f0d6d0656c72de61 100644
--- a/install/ui/aci.js
+++ b/install/ui/aci.js
@@ -23,11 +23,15 @@
 
 /* REQUIRES: ipa.js, details.js, search.js, add.js, facet.js, entity.js */
 
-IPA.entity_factories.permission = function() {
+IPA.aci = {};
 
-return IPA.entity_builder().
-entity('permission').
-facet_groups([ 'privilege' , 'settings' ]).
+IPA.aci.permission_entity = function(spec) {
+
+var that = IPA.entity(spec);
+
+that.init = function(params) {
+
+params.builder.facet_groups([ 'privilege' , 'settings' ]).
 search_facet({
 columns:['cn']
 }).
@@ -78,15 +82,19 @@ IPA.entity_factories.permission = function() {
 label: IPA.messages.objects.permission.target
 }
 ]
-}).
-build();
+});
+};
+
+return that;
 };
 
+IPA.aci.privilege_entity = function(spec) {
 
-IPA.entity_factories.privilege = function() {
-return IPA.entity_builder().
-entity('privilege').
-facet_groups([ 'role', 'settings', 'permission' ]).
+var that = IPA.entity(spec);
+
+that.init = function(params) {
+
+params.builder.facet_groups([ 'role', 'settings', 'permission' ]).
 search_facet({
 columns: [
 'cn',
@@ -130,16 +138,19 @@ IPA.entity_factories.privilege = function() {
 name: 'description'
 }
 ]
-}).
-build();
+});
+};
 
+return that;
 };
 
+IPA.aci.role_entity = function(spec) {
 
-IPA.entity_factories.role = function() {
-return  IPA.entity_builder().
-entity('role').
-facet_groups([ 'member', 'settings', 'privilege' ]).
+var that = IPA.entity(spec);
+
+that.init = function(params) {
+
+params.builder.facet_groups([ 'member', 'settings', 'privilege' ]).
 search_facet({
 columns: [
 'cn',
@@ -176,15 +187,19 @@ IPA.entity_factories.role = function() {
 name: 'description'
 }
 ]
-}).
-build();
+});
+};
+
+return that;
 };
 
+IPA.aci.selfservice_entity = function(spec) {
 
-IPA.entity_factories.selfservice = function() {
-return IPA.entity_builder().
-entity('selfservice').
-search_facet({
+var that = IPA.entity(spec);
+
+that.init = function(params) {
+
+params.builder.search_facet({
 columns:['aciname']}).
 details_facet({
 sections:[{
@@ -204,15 +219,19 @@ IPA.entity_factories.selfservice = function() {
  object_type:'user',
  name:'attrs'
 }]
-}).
-build();
+});
+};
+
+return that;
 };
 
+IPA.aci.delegation_entity = function(spec) {
 
-IPA.entity_factories.delegation = function() {
-return IPA.entity_builder().
-entity('delegation').
-search_facet({
+var that = IPA.entity(spec);
+
+that.init = function(params) {
+
+params.builder.search_facet({
 columns:['aciname']}).

Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib

Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib

Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib

[Freeipa-devel] [PATCH] 158 Fix ipa-server-install answer cache

Re: [Freeipa-devel] [PATCH] 1 Do lazy initializiation ipalib

[Freeipa-devel] [PATCH] 030 Extending facet's mechanism of gathering changes

Re: [Freeipa-devel] Unifying the PKI and IPA Directory Server instances

Re: [Freeipa-devel] Unifying the PKI and IPA Directory Server instances

[Freeipa-devel] [PATCH] fix memleak in ipa-kdb

[Freeipa-devel] [PATCH] 031 Field for DNS SOA class changed to combobox with options

Re: [Freeipa-devel] Unifying the PKI and IPA Directory Server instances

Re: [Freeipa-devel] Unifying the PKI and IPA Directory Server instances

Re: [Freeipa-devel] Unifying the PKI and IPA Directory Server instances

Re: [Freeipa-devel] Unifying the PKI and IPA Directory Server instances

Re: [Freeipa-devel] Unifying the PKI and IPA Directory Server instances

Re: [Freeipa-devel] Unifying the PKI and IPA Directory Server instances

[Freeipa-devel] [PATCHES] #2036 Fix coverity bugs

[Freeipa-devel] [PATCHES] #2037 Coverity issues

Re: [Freeipa-devel] [PATCH] 158 Fix ipa-server-install answer cache

Re: [Freeipa-devel] [PATCH] 029 Page is cleared before it is visible

Re: [Freeipa-devel] [PATCH] 158 Fix ipa-server-install answer cache

[Freeipa-devel] [PATCH] #2038 modify salt creation

Re: [Freeipa-devel] [PATCH] 030 Extending facet's mechanism of gathering changes

Re: [Freeipa-devel] [PATCH] 031 Field for DNS SOA class changed to combobox with options

[Freeipa-devel] [PATCH] 307 Added extensible UI framework.

25 matches

Site Navigation

Mail list logo

Footer information