Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable

2014-10-29 Thread Martin Kosek
On 10/28/2014 09:59 PM, Nathaniel McCallum wrote:
 On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote:
 This patch gives the administrator variables to control the size of
 the authentication and synchronization windows for OTP tokens.

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

 NOTE: There is one known issue with this patch which I don't know how to
 solve. This patch changes the schema in install/share/60ipaconfig.ldif.
 On an upgrade, all of the new attributeTypes appear correctly. However,
 the modifications to the pre-existing objectClass do not show up on the
 server. What am I doing wrong?

 After modifying ipaGuiConfig manually, everything in this patch works
 just fine.
 
 This new version takes into account the new (proper) OIDs and attribute
 names.

Thanks Nathaniel!

 The above known issue still remains.

Petr3, any idea what could have gone wrong? ObjectClass MAY list extension
should work just fine, AFAIK.

Martin

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


Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable

2014-10-29 Thread Petr Viktorin

On 10/29/2014 10:37 AM, Martin Kosek wrote:

On 10/28/2014 09:59 PM, Nathaniel McCallum wrote:

On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote:

This patch gives the administrator variables to control the size of
the authentication and synchronization windows for OTP tokens.

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

NOTE: There is one known issue with this patch which I don't know how to
solve. This patch changes the schema in install/share/60ipaconfig.ldif.
On an upgrade, all of the new attributeTypes appear correctly. However,
the modifications to the pre-existing objectClass do not show up on the
server. What am I doing wrong?

After modifying ipaGuiConfig manually, everything in this patch works
just fine.


This new version takes into account the new (proper) OIDs and attribute
names.


Thanks Nathaniel!


The above known issue still remains.


Petr3, any idea what could have gone wrong? ObjectClass MAY list extension
should work just fine, AFAIK.


You added a blank line to the LDIF file. This is an entry separator, so 
the objectClasses after the blank line don't belong to cn=schema, so 
they aren't considered in the update.

Without the blank line it works fine.

--
Petr³

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

Re: [Freeipa-devel] [PATCH] 333 Handle profile changes in dogtag-ipa-ca-renew-agent

2014-10-29 Thread David Kupka

On 10/14/2014 10:52 AM, Jan Cholasta wrote:

Hi,

the attached patch fixes https://fedorahosted.org/freeipa/ticket/4627.

(The original patch was posted at
http://www.redhat.com/archives/freeipa-devel/2014-September/msg00454.html.)


How to test:

  1. install server

  2. run ipa-certupdate

  3. run getcert list -d /etc/pki/pki-tomcat/alias -n 'caSigningCert
cert-pki-ca', the request should be in CA_WORKING state

  4. run ipa-cacert-manage renew

  5. run getcert list -d /etc/pki/pki-tomcat/alias -n 'caSigningCert
cert-pki-ca', the request should be in MONITORING state, there should
be no ca-error

Honza



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



Works for me, ACK.
--
David Kupka

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


Re: [Freeipa-devel] [PATCH, slapi-nis] ID view-related patches to slapi-nis

2014-10-29 Thread thierry bordaz

On 10/28/2014 10:11 PM, Alexander Bokovoy wrote:

Hi,

two patches to slapi-nis are attached:

- make sure only DNs from the schema-compat trees are targeted for ID
 view replacement. This solves issue of 
https://bugzilla.redhat.com/show_bug.cgi?id=1157989

 found by Sumit.

- support ID overrides in the BIND callback. So far the only thing we
 need is overriding uid.

They need to be applied in this order, on top of 0.54 release version of
slapi-nis.



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

Hi Alexander,

The patches fixed the test case in 
https://bugzilla.redhat.com/show_bug.cgi?id=1157989.

Few comments regarding the patch:

 * in backend_search_cb, it checks if the search is in one of the
   container. We need that cbdata.answer=FALSE at the end of the checking.
   Why not setting it systematically at the end.
 * in backend_locate, 'target' is a duplicate of cbdata.target. But
   then when calling idview_replace_target_dn it may be changed.
   Will not it lead to a leak ?


Thanks
thierry

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

Re: [Freeipa-devel] [PATCH] 352 Fixed KRA backend.

2014-10-29 Thread Petr Viktorin

On 10/28/2014 10:51 PM, Endi Sukma Dewata wrote:

Thanks for the review. New patch attached.

On 10/23/2014 3:59 AM, Petr Viktorin wrote:

In IPA we usually include the full ticket URL, not just the number.


Fixed.


The build fails with a lint message:
* Module ipaserver.plugins.dogtag
ipaserver/plugins/dogtag.py:1903: [E1123(unexpected-keyword-arg),
kra.get_client] Unexpected keyword argument 'password_file' in
constructor call)
ipaserver/plugins/dogtag.py:1903: [E1120(no-value-for-parameter),
kra.get_client] No value for argument 'certdb_password' in constructor
call)

I have pki-base-10.2.0-3.fc21.noarch, where NSSCryptoProvider indeed
takes password and not password_file. If a newer version is required you
should put it in the spec.


Fixed. Dependency is bumped to 10.2.1-0.1 which is available from my
COPR repo:

   dnf copr enable edewata/pki


OK. We should get that to an IPA COPR before merging this.


ipaserver.install.certs.CertDB.install_pem_from_p12:
If p12_passwd is missing and pwd_fname is None, this will crash.
Please document how the method should be called. And assert that exactly
one of p12_passwd and pwd_fname is given.


I reverted this change because the KRA backend actually no longer uses
install_pem_from_p12(). The KRA backend is now using the CLI from the
new Dogtag which generates the proper PEM format for client
authentication, so I'll leave install_pem_from_p12() unmodified because
it's still used by KrbInstance.


ipaserver.plugins.dogtag.kra.get_client:
Should every caller check if this returns None?
If not, raise an exception instead.
If yes, at least mention it in a docstring.


Fixed. It's now raising a generic exception.

Is there an existing exception that is more appropriate for backend
issues like this?


I'd go for RuntimeError.
Don't use translatable strings (the _ function) if you're not using 
ipalib.PublicError subclasses.





Typo in commit message: modified to use Dogtag's CLI *go* create


Fixed.




How can I do some basic smoke check on this? Is there something I still 
need to to besides ipa-kra-istall? Any other patches?

I tried:

from ipalib import api
from pki.key import KeyClient
api.bootstrap(context='server')
api.finalize()
keyclient = api.Backend.kra.get_client()
keyclient.keys.archive_key('test3', KeyClient.PASS_PHRASE_TYPE, 'tkey')

which gives me:

Traceback (most recent call last):
  File stdin, line 1, in module
  File /usr/lib/python2.7/site-packages/pki/__init__.py, line 295, in 
handler

return fn_call(inst, *args, **kwargs)
  File /usr/lib/python2.7/site-packages/pki/key.py, line 687, in 
archive_key

nonce_iv = self.crypto.generate_nonce_iv()
  File /usr/lib/python2.7/site-packages/pki/crypto.py, line 176, in 
generate_nonce_iv

iv_data = nss.generate_random(iv_length)
nss.error.NSPRError: (SEC_ERROR_NO_TOKEN) The security card or token 
does not exist, needs to be initialized, or has been removed.



--
Petr³

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


Re: [Freeipa-devel] [PATCH 0020] Fix zone name to directory name conversion in BINDMgr

2014-10-29 Thread Martin Basti

On 23/10/14 15:44, Martin Basti wrote:

On 23/10/14 14:15, Petr Spacek wrote:

Hello,

Fix zone name to directory name conversion in BINDMgr.

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


ACK, signing the root zone works fine now


The patch is ready to be pushed.

--
Martin Basti

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


Re: [Freeipa-devel] [PATCH] 0025 Respect UID and GID soft static allocation.

2014-10-29 Thread David Kupka

On 10/24/2014 03:05 PM, David Kupka wrote:

On 10/24/2014 01:06 PM, David Kupka wrote:

On 10/24/2014 10:43 AM, Martin Basti wrote:

On 24/10/14 09:51, David Kupka wrote:

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

NACK

1)
Why is there line with 'DS System User?' The comment should depend on
service.

+args = [
+paths.USERADD,
+'-g', group,
+'-c', 'DS System User',
+'-d', homedir,
+'-s', shell,
+'-M', '-r', name,
+]


This was part of the original code and I didn't notice it. Nice catch,
thanks.



2)
code create_system_user is duplicated between base and redhat tasks with
platform dependent changes.
IMO it would be better to have one method to create user, with keyword
arguments.  And then platform dependent method which will call method to
create user with appropriate arguments (or with default arguments)



You're right it was ugly.



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


I shouldn't break SOLID principles.



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


Using super is probably better that explicit naming of parent class.
Let user (developer) override UID/GID and hope that he knows why ...
--
David Kupka
From da0e375ac81d297a78649de7be98e8610aa83dcc Mon Sep 17 00:00:00 2001
From: David Kupka dku...@redhat.com
Date: Wed, 22 Oct 2014 09:07:44 -0400
Subject: [PATCH] Respect UID and GID soft static allocation.

https://fedoraproject.org/wiki/Packaging:UsersAndGroups?rd=Packaging/UsersAndGroups#Soft_static_allocation

https://fedorahosted.org/freeipa/ticket/4585
---
 ipaplatform/base/tasks.py | 48 +++
 ipaplatform/redhat/tasks.py   | 21 +
 ipaserver/install/cainstance.py   |  2 +-
 ipaserver/install/dsinstance.py   |  2 +-
 ipaserver/install/installutils.py | 42 --
 5 files changed, 71 insertions(+), 44 deletions(-)

diff --git a/ipaplatform/base/tasks.py b/ipaplatform/base/tasks.py
index 408447e43cd36d0cdf11a1877b3bc9880c4785de..f2ba81f44bb991b218232aad84d7810cdae839ef 100644
--- a/ipaplatform/base/tasks.py
+++ b/ipaplatform/base/tasks.py
@@ -22,7 +22,13 @@
 This module contains default platform-specific implementations of system tasks.
 '''
 
+import pwd
+import grp
 from ipaplatform.paths import paths
+from ipapython.ipa_log_manager import log_mgr
+from ipapython import ipautil
+
+log = log_mgr.get_logger(__name__)
 
 
 class BaseTaskNamespace(object):
@@ -150,5 +156,47 @@ class BaseTaskNamespace(object):
 
 return
 
+def create_system_user(self, name, group, homedir, shell, uid = None, gid = None, comment = None):
+Create a system user with a corresponding group
+try:
+grp.getgrnam(group)
+except KeyError:
+log.debug('Adding group %s', group)
+args = [paths.GROUPADD, '-r', group]
+if gid:
+args += ['-g', str(gid)]
+try:
+ipautil.run(args)
+log.debug('Done adding group')
+except ipautil.CalledProcessError as e:
+log.critical('Failed to add group: %s', e)
+raise
+else:
+log.debug('group %s exists', group)
+
+try:
+pwd.getpwnam(name)
+except KeyError:
+log.debug('Adding user %s', name)
+args = [
+paths.USERADD,
+'-g', group,
+'-d', homedir,
+'-s', shell,
+'-M', '-r', name,
+]
+if uid:
+args += ['-u', str(uid)]
+if comment:
+args += ['-c', comment]
+try:
+ipautil.run(args)
+log.debug('Done adding user')
+except ipautil.CalledProcessError as e:
+log.critical('Failed to add user: %s', e)
+raise
+else:
+log.debug('user %s exists', name)
+
 
 task_namespace = BaseTaskNamespace()
diff --git a/ipaplatform/redhat/tasks.py b/ipaplatform/redhat/tasks.py
index 16d90a6d1a7d3d9aced5de82a5c1efe6b8c2..7c03fe7cebaa7a5c5ac8715fe23991be7570ea75 100644
--- a/ipaplatform/redhat/tasks.py
+++ b/ipaplatform/redhat/tasks.py
@@ -393,5 +393,26 @@ class RedHatTaskNamespace(BaseTaskNamespace):
 
 return True
 
+def create_system_user(self, name, group, homedir, shell, uid = None, gid = None, comment = None):
+
+Create a system user with a corresponding group
+
+According to https://fedoraproject.org/wiki/Packaging:UsersAndGroups?rd=Packaging/UsersAndGroups#Soft_static_allocation
+some system users should have fixed UID, GID and other 

Re: [Freeipa-devel] [PATCH 0074] Make token window sizes configurable

2014-10-29 Thread Nathaniel McCallum
On Wed, 2014-10-29 at 12:21 +0100, Petr Viktorin wrote:
 On 10/29/2014 10:37 AM, Martin Kosek wrote:
  On 10/28/2014 09:59 PM, Nathaniel McCallum wrote:
  On Thu, 2014-10-23 at 18:07 -0400, Nathaniel McCallum wrote:
  This patch gives the administrator variables to control the size of
  the authentication and synchronization windows for OTP tokens.
 
  https://fedorahosted.org/freeipa/ticket/4511
 
  NOTE: There is one known issue with this patch which I don't know how to
  solve. This patch changes the schema in install/share/60ipaconfig.ldif.
  On an upgrade, all of the new attributeTypes appear correctly. However,
  the modifications to the pre-existing objectClass do not show up on the
  server. What am I doing wrong?
 
  After modifying ipaGuiConfig manually, everything in this patch works
  just fine.
 
  This new version takes into account the new (proper) OIDs and attribute
  names.
 
  Thanks Nathaniel!
 
  The above known issue still remains.
 
  Petr3, any idea what could have gone wrong? ObjectClass MAY list extension
  should work just fine, AFAIK.
 
 You added a blank line to the LDIF file. This is an entry separator, so 
 the objectClasses after the blank line don't belong to cn=schema, so 
 they aren't considered in the update.
 Without the blank line it works fine.

Thanks for the catch!

Here is a version without the blank line.
From 6402e1f50885af226db35495063d8b50cf246300 Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum npmccal...@redhat.com
Date: Thu, 23 Oct 2014 15:18:26 -0400
Subject: [PATCH] Make token window sizes configurable

This patch gives the administrator variables to control the size of
the authentication and synchronization windows for OTP tokens.

https://fedorahosted.org/freeipa/ticket/4511
---
 API.txt   |   6 +-
 VERSION   |   4 +-
 daemons/ipa-slapi-plugins/ipa-pwd-extop/authcfg.c | 195 +-
 daemons/ipa-slapi-plugins/ipa-pwd-extop/authcfg.h |  17 ++
 daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c |  77 +++--
 daemons/ipa-slapi-plugins/ipa-pwd-extop/syncreq.c |   5 +-
 daemons/ipa-slapi-plugins/libotp/libotp.c | 133 +++
 daemons/ipa-slapi-plugins/libotp/libotp.h |  30 ++--
 install/share/60ipaconfig.ldif|   6 +-
 install/ui/src/freeipa/serverconfig.js|  10 ++
 install/ui/test/data/ipa_init.json|   3 +-
 install/updates/40-otp.update |   6 +
 ipalib/plugins/config.py  |  31 +++-
 ipalib/plugins/internal.py|   1 +
 14 files changed, 333 insertions(+), 191 deletions(-)

diff --git a/API.txt b/API.txt
index 491d7a76fd1d2d50208d314d1600839ce295..4f204d0fa2e33dc4c9202645e111c25d2a545d70 100644
--- a/API.txt
+++ b/API.txt
@@ -514,7 +514,7 @@ args: 0,1,1
 option: Str('version?', exclude='webui')
 output: Output('result', None, None)
 command: config_mod
-args: 0,25,3
+args: 0,29,3
 option: Str('addattr*', cli_name='addattr', exclude='webui')
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
 option: Str('delattr*', cli_name='delattr', exclude='webui')
@@ -525,6 +525,8 @@ option: Str('ipadefaultprimarygroup', attribute=True, autofill=False, cli_name='
 option: Str('ipagroupobjectclasses', attribute=True, autofill=False, cli_name='groupobjectclasses', csv=True, multivalue=True, required=False)
 option: IA5Str('ipagroupsearchfields', attribute=True, autofill=False, cli_name='groupsearch', multivalue=False, required=False)
 option: IA5Str('ipahomesrootdir', attribute=True, autofill=False, cli_name='homedirectory', multivalue=False, required=False)
+option: Int('ipahotpauthwindow', attribute=True, autofill=False, cli_name='hotp_auth_window', maxvalue=1000, minvalue=1, multivalue=False, required=False)
+option: Int('ipahotpsyncwindow', attribute=True, autofill=False, cli_name='hotp_sync_window', maxvalue=1000, minvalue=1, multivalue=False, required=False)
 option: StrEnum('ipakrbauthzdata', attribute=True, autofill=False, cli_name='pac_type', csv=True, multivalue=True, required=False, values=(u'MS-PAC', u'PAD', u'nfs:NONE'))
 option: Int('ipamaxusernamelength', attribute=True, autofill=False, cli_name='maxusername', minvalue=1, multivalue=False, required=False)
 option: Bool('ipamigrationenabled', attribute=True, autofill=False, cli_name='enable_migration', multivalue=False, required=False)
@@ -533,6 +535,8 @@ option: Int('ipasearchrecordslimit', attribute=True, autofill=False, cli_name='s
 option: Int('ipasearchtimelimit', attribute=True, autofill=False, cli_name='searchtimelimit', minvalue=-1, multivalue=False, required=False)
 option: Str('ipaselinuxusermapdefault', attribute=True, autofill=False, cli_name='ipaselinuxusermapdefault', multivalue=False, required=False)
 option: Str('ipaselinuxusermaporder', attribute=True, autofill=False, cli_name='ipaselinuxusermaporder', multivalue=False, 

Re: [Freeipa-devel] [PATCH 0020] Fix zone name to directory name conversion in BINDMgr

2014-10-29 Thread Petr Viktorin

On 10/29/2014 02:01 PM, Martin Basti wrote:

On 23/10/14 15:44, Martin Basti wrote:

On 23/10/14 14:15, Petr Spacek wrote:

Hello,

Fix zone name to directory name conversion in BINDMgr.

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


ACK, signing the root zone works fine now


The patch is ready to be pushed.



Pushed to:
master: ac53fda7142c4c0bb27cd5d1e5aea105f777
ipa-4-1: 4e42d171300114bffc62ac12bbe1c00f7c8f4518

--
Petr³

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


Re: [Freeipa-devel] [PATCH] 333 Handle profile changes in dogtag-ipa-ca-renew-agent

2014-10-29 Thread Petr Viktorin

On 10/29/2014 12:37 PM, David Kupka wrote:

On 10/14/2014 10:52 AM, Jan Cholasta wrote:

Hi,

the attached patch fixes https://fedorahosted.org/freeipa/ticket/4627.

(The original patch was posted at
http://www.redhat.com/archives/freeipa-devel/2014-September/msg00454.html.)



How to test:

  1. install server

  2. run ipa-certupdate

  3. run getcert list -d /etc/pki/pki-tomcat/alias -n 'caSigningCert
cert-pki-ca', the request should be in CA_WORKING state

  4. run ipa-cacert-manage renew

  5. run getcert list -d /etc/pki/pki-tomcat/alias -n 'caSigningCert
cert-pki-ca', the request should be in MONITORING state, there should
be no ca-error

Honza



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



Works for me, ACK.


Pushed to:
master: a649a84a1bd7eb3c727fdcfc341b326a19b0ee5a
ipa-4-1: 2ee248bd7efc3559d4420751f8ee34b270b36b49

--
Petr³

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


Re: [Freeipa-devel] [PATCH, slapi-nis] ID view-related patches to slapi-nis

2014-10-29 Thread thierry bordaz

On 10/29/2014 02:23 PM, Alexander Bokovoy wrote:

On Wed, 29 Oct 2014, thierry bordaz wrote:
The patches fixed the test case in 
https://bugzilla.redhat.com/show_bug.cgi?id=1157989.

Few comments regarding the patch:

* in backend_search_cb, it checks if the search is in one of the
  container. We need that cbdata.answer=FALSE at the end of the 
checking.

  Why not setting it systematically at the end.

I've moved it to the end of the block.


* in backend_locate, 'target' is a duplicate of cbdata.target. But
  then when calling idview_replace_target_dn it may be changed.
  Will not it lead to a leak ?

Good catch, thanks!

Fixed version attached.


Hi Alexander,

The patches are good to me. Ack

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

Re: [Freeipa-devel] [PATCHES] 0661-0672 Switch the test suite to pytest

2014-10-29 Thread Petr Viktorin

On 10/29/2014 01:22 PM, Tomas Babej wrote:


On 10/27/2014 04:38 PM, Petr Viktorin wrote:

On 10/15/2014 02:58 PM, Petr Viktorin wrote:

This almost completes the switch to pytest. There are two missing
things:
- the details of test results (--with-xunit) are not read correctly by
Jenkins. I have a theory I'm investigating here.
- the beakerlib integration is still not ready


I'll not be available for the rest of the week so I'm sending this
early, in case someone wants to take a look.


I've updated (and rearranged) the patches after some more testing.
Both points above are fixed. Individual plugins are broken out; some
would be nice to even release independently of IPA. (There is some
demand for the BeakerLib plugin; for that I'd only need to break the
dependency on ipa_log_manager.)


These depend on my patches 0656-0660.




Thanks for this effort!

 Patch 0656: tests: Use PEP8-compliant setup/teardown method names

There are some references to the old names in the test_ipapython and
test_install:

 [tbabej@thinkpad7 ipatests]$ git grep setUpClass
 [tbabej@thinkpad7 ipatests]$ git grep tearDownClass
 [tbabej@thinkpad7 ipatests]$ git grep setUp
 test_install/test_updates.py:def setUp(self):
 test_ipapython/test_cookie.py:def setUp(self):
 test_ipapython/test_cookie.py:def setUp(self):
 test_ipapython/test_cookie.py:def setUp(self):
 test_ipapython/test_dn.py:def setUp(self):
 test_ipapython/test_dn.py:def setUp(self):
 test_ipapython/test_dn.py:def setUp(self):
 test_ipapython/test_dn.py:def setUp(self):
 test_ipapython/test_dn.py:def setUp(self):
 [tbabej@thinkpad7 ipatests]$ git grep tearDown
 test_install/test_updates.py:def tearDown(self):


These are in unittest.testCase. It would be nice to convert those as 
well, but that's for a larger cleanup.



 Patch 0657: tests: Add configuration for pytest

Shouldn't we rather change the class names?


Ideally yes, but I don't think renaming most of our test classes would 
be worth the benefit.



 Patch 0658: ipatests.util.ClassChecker: Raise AttributeError in
get_subcls

ACK.

 Patch 0659: test_automount_plugin: Fix test ordering

ACK.

 PATCH 0660: Use setup_class/teardown_class in Declarative tests

 --- a/ipatests/test_ipaserver/test_changepw.py
 +++ b/ipatests/test_ipaserver/test_changepw.py
 @@ -33,8 +33,7 @@
  class test_changepw(XMLRPC_test, Unauthorized_HTTP_test):
  app_uri = '/ipa/session/change_password'

 -def setup(self):
 -super(test_changepw, self).setup()
 +def setup(cls):
  try:
  api.Command['user_add'](uid=testuser,
givenname=u'Test', sn=u'User')
  api.Command['passwd'](testuser, password=u'old_password')
 @@ -43,12 +42,11 @@ def setup(self):
  'Cannot set up test user: %s' % e
  )

 -def teardown(self):
 +def teardown(cls):
  try:
  api.Command['user_del']([testuser])
  except errors.NotFound:
  pass
 -super(test_changepw, self).teardown()

The setup/teardown methods are not classmethods, so the name of the
first argument should remain self.


Oops, thanks for the catch!


 PATCH 0661: dogtag plugin: Don't use doctest syntax for non-doctest
examples

ACK.

 PATCH 0662: test_webui: Don't use __init__ for test classes

I don't think the following change will work:

 -def __init__(self, driver=None, config=None):
 +def setup(self, driver=None, config=None):
  self.request_timeout = 30
  self.driver = driver
  self.config = config
  if not config:
  self.load_config()
 +self.get_driver().maximize_window()
 +
 +def teardown(self):
 +self.driver.quit()

  def load_config(self):
  
 @@ -161,20 +165,6 @@ def load_config(self):
  if 'type' not in c:
  c['type'] = DEFAULT_TYPE

 -def setup(self):
 -
 -Test setup
 -
 -if not self.driver:
 -self.driver = self.get_driver()
 -self.driver.maximize_window()
 -
 -def teardown(self):
 -
 -Test clean up
 -
 -self.driver.quit()


The setup(self) method used to set the self.driver attribute on the
class instance, now nothing sets it. The setup method should do:

 def setup(self, driver=None, config=None):
 self.request_timeout = 30
 self.driver = driver
 self.config = config
 if not config:
 self.load_config()
 if not self.driver:
 self.driver = self.get_driver()
 self.driver.maximize_window()

However, I would prefer having self.driver as a cached property.



Re: [Freeipa-devel] [PATCHES] 0661-0672 Switch the test suite to pytest

2014-10-29 Thread Petr Viktorin

Please ignore the previous mail, I hit Send my mistake.

--
Petr³

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


Re: [Freeipa-devel] [PATCH] 0025 Respect UID and GID soft static allocation.

2014-10-29 Thread David Kupka

On 10/29/2014 02:34 PM, David Kupka wrote:

On 10/24/2014 03:05 PM, David Kupka wrote:

On 10/24/2014 01:06 PM, David Kupka wrote:

On 10/24/2014 10:43 AM, Martin Basti wrote:

On 24/10/14 09:51, David Kupka wrote:

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

NACK

1)
Why is there line with 'DS System User?' The comment should depend on
service.

+args = [
+paths.USERADD,
+'-g', group,
+'-c', 'DS System User',
+'-d', homedir,
+'-s', shell,
+'-M', '-r', name,
+]


This was part of the original code and I didn't notice it. Nice catch,
thanks.



2)
code create_system_user is duplicated between base and redhat tasks
with
platform dependent changes.
IMO it would be better to have one method to create user, with keyword
arguments.  And then platform dependent method which will call
method to
create user with appropriate arguments (or with default arguments)



You're right it was ugly.



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


I shouldn't break SOLID principles.



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


Using super is probably better that explicit naming of parent class.
Let user (developer) override UID/GID and hope that he knows why ...


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



--
David Kupka
From ec277c8d8d9fc66f31236f306e038d39aa7417fb Mon Sep 17 00:00:00 2001
From: David Kupka dku...@redhat.com
Date: Wed, 22 Oct 2014 09:07:44 -0400
Subject: [PATCH] Respect UID and GID soft static allocation.

https://fedoraproject.org/wiki/Packaging:UsersAndGroups?rd=Packaging/UsersAndGroups#Soft_static_allocation

https://fedorahosted.org/freeipa/ticket/4585
---
 ipaplatform/base/tasks.py | 48 +++
 ipaplatform/redhat/tasks.py   | 23 +++
 ipaserver/install/cainstance.py   |  2 +-
 ipaserver/install/dsinstance.py   |  2 +-
 ipaserver/install/installutils.py | 42 --
 5 files changed, 73 insertions(+), 44 deletions(-)

diff --git a/ipaplatform/base/tasks.py b/ipaplatform/base/tasks.py
index 408447e43cd36d0cdf11a1877b3bc9880c4785de..f2ba81f44bb991b218232aad84d7810cdae839ef 100644
--- a/ipaplatform/base/tasks.py
+++ b/ipaplatform/base/tasks.py
@@ -22,7 +22,13 @@
 This module contains default platform-specific implementations of system tasks.
 '''
 
+import pwd
+import grp
 from ipaplatform.paths import paths
+from ipapython.ipa_log_manager import log_mgr
+from ipapython import ipautil
+
+log = log_mgr.get_logger(__name__)
 
 
 class BaseTaskNamespace(object):
@@ -150,5 +156,47 @@ class BaseTaskNamespace(object):
 
 return
 
+def create_system_user(self, name, group, homedir, shell, uid = None, gid = None, comment = None):
+Create a system user with a corresponding group
+try:
+grp.getgrnam(group)
+except KeyError:
+log.debug('Adding group %s', group)
+args = [paths.GROUPADD, '-r', group]
+if gid:
+args += ['-g', str(gid)]
+try:
+ipautil.run(args)
+log.debug('Done adding group')
+except ipautil.CalledProcessError as e:
+log.critical('Failed to add group: %s', e)
+raise
+else:
+log.debug('group %s exists', group)
+
+try:
+pwd.getpwnam(name)
+except KeyError:
+log.debug('Adding user %s', name)
+args = [
+paths.USERADD,
+'-g', group,
+'-d', homedir,
+'-s', shell,
+'-M', '-r', name,
+]
+if uid:
+args += ['-u', str(uid)]
+if comment:
+args += ['-c', comment]
+try:
+ipautil.run(args)
+log.debug('Done adding user')
+except ipautil.CalledProcessError as e:
+log.critical('Failed to add user: %s', e)
+raise
+else:
+log.debug('user %s exists', name)
+
 
 task_namespace = BaseTaskNamespace()
diff --git a/ipaplatform/redhat/tasks.py b/ipaplatform/redhat/tasks.py
index 16d90a6d1a7d3d9aced5de82a5c1efe6b8c2..3f5fc90b4b988d44cf0dcaa5a8d569a3c5f453ee 100644
--- a/ipaplatform/redhat/tasks.py
+++ b/ipaplatform/redhat/tasks.py
@@ -393,5 +393,28 @@ class RedHatTaskNamespace(BaseTaskNamespace):
 
 return True
 
+def create_system_user(self, name, group, homedir, shell, uid = None, gid = None, comment = None):
+
+Create a system user with a corresponding 

Re: [Freeipa-devel] [PATCH] 352 Fixed KRA backend.

2014-10-29 Thread Endi Sukma Dewata

New patch attached.

On 10/29/2014 7:58 AM, Petr Viktorin wrote:

Dependency is bumped to 10.2.1-0.1 which is available from my
COPR repo:

   dnf copr enable edewata/pki


OK. We should get that to an IPA COPR before merging this.


How do we do that? Here is the SRPM:
https://edewata.fedorapeople.org/pki/copr/pki-core-10.2.1-0.1.fc20.src.rpm


ipaserver.plugins.dogtag.kra.get_client:
Should every caller check if this returns None?
If not, raise an exception instead.
If yes, at least mention it in a docstring.


Fixed. It's now raising a generic exception.

Is there an existing exception that is more appropriate for backend
issues like this?


I'd go for RuntimeError.


Fixed.


Don't use translatable strings (the _ function) if you're not using
ipalib.PublicError subclasses.


Fixed.


How can I do some basic smoke check on this? Is there something I still
need to to besides ipa-kra-istall? Any other patches?
I tried:

from ipalib import api
from pki.key import KeyClient
api.bootstrap(context='server')
api.finalize()
keyclient = api.Backend.kra.get_client()
keyclient.keys.archive_key('test3', KeyClient.PASS_PHRASE_TYPE, 'tkey')

which gives me:

Traceback (most recent call last):
   File stdin, line 1, in module
   File /usr/lib/python2.7/site-packages/pki/__init__.py, line 295, in
handler
 return fn_call(inst, *args, **kwargs)
   File /usr/lib/python2.7/site-packages/pki/key.py, line 687, in
archive_key
 nonce_iv = self.crypto.generate_nonce_iv()
   File /usr/lib/python2.7/site-packages/pki/crypto.py, line 176, in
generate_nonce_iv
 iv_data = nss.generate_random(iv_length)
nss.error.NSPRError: (SEC_ERROR_NO_TOKEN) The security card or token
does not exist, needs to be initialized, or has been removed.


The simplest test is probably this:

from ipalib import api

api.bootstrap(context='server')
api.finalize()

kra_client = api.Backend.kra.get_client()
transport_cert = kra_client.system_certs.get_transport_cert()

print Serial number: %s % transport_cert.serial_number
print Issuer DN: %s % transport_cert.issuer_dn
print Subject DN: %s % transport_cert.subject_dn

print transport_cert.encoded

If you want to test the key archival it would require installing a 
transport certificate and add some authentication operations. A better 
way to do that is to install patch #354-1, #353-3, #355-1, and #356-1 
and test the vault-archive command. It will install the transport cert 
automatically and perform the required authentication.


--
Endi S. Dewata
From 7b68ff1c93554975abd75e6d672ee18a7a0bcf04 Mon Sep 17 00:00:00 2001
From: Endi S. Dewata edew...@redhat.com
Date: Wed, 1 Oct 2014 14:59:46 -0400
Subject: [PATCH] Fixed KRA backend.

The KRA backend has been simplified since most of the tasks have
been moved somewhere else. The transport certificate will be
installed on the client, and it is not needed by KRA backend. The
KRA agent's PEM certificate is now generated during installation
due to permission issue. The kra_host() for now is removed since
the current ldap_enable() cannot register the KRA service, so it
is using the kra_host environment variable.

The KRA installer has been modified to use Dogtag's CLI to create
KRA agent and setup the client authentication.

The proxy settings have been updated to include KRA's URLs.

Some constants have been renamed for clarity. The DOGTAG_AGENT_P12
has been renamed to DOGTAG_ADMIN_P12 since file actually contains
the Dogtag admin's certificate and private key and it can be used
to access both CA and KRA. The DOGTAG_AGENT_PEM has been renamed
to KRA_AGENT_PEM since it can only be used for KRA.

The Dogtag dependency has been updated to 10.2.1-0.1.

https://fedorahosted.org/freeipa/ticket/4503
---
 freeipa.spec.in  |   4 +-
 install/conf/ipa-pki-proxy.conf  |   2 +-
 ipaplatform/base/paths.py|   4 +-
 ipaserver/install/cainstance.py  |   4 +-
 ipaserver/install/ipa_backup.py  |   3 +-
 ipaserver/install/krainstance.py |  83 +++---
 ipaserver/plugins/dogtag.py  | 122 ++-
 7 files changed, 101 insertions(+), 121 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 
8fcb535e229db4f7a8eaaee3c99b18446eef7f1e..dc04be48b2bb52ff05f9fab371c4b333a15d24ca
 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -130,8 +130,8 @@ Requires(post): systemd-units
 Requires: selinux-policy = %{selinux_policy_version}
 Requires(post): selinux-policy-base
 Requires: slapi-nis = 0.54-1
-Requires: pki-ca = 10.2.0-3
-Requires: pki-kra = 10.2.0
+Requires: pki-ca = 10.2.1-0.1
+Requires: pki-kra = 10.2.1-0.1
 %if 0%{?rhel}
 Requires: subscription-manager
 %endif
diff --git a/install/conf/ipa-pki-proxy.conf b/install/conf/ipa-pki-proxy.conf
index 
2370b4d7a7467a7e47c0d223915e018c9a009e83..5d21156848f3b5ddf14c42d92a26a30a9f94af36
 100644
--- a/install/conf/ipa-pki-proxy.conf
+++ b/install/conf/ipa-pki-proxy.conf
@@ -19,7 +19,7 @@ ProxyRequests Off
 /LocationMatch
 
 # matches for agent