Re: [Freeipa-devel] [PATCH 0003] Test validity of URIs in certificate
On 11.08.2016 08:40, Lenka Doudova wrote: On 08/10/2016 05:48 PM, Martin Basti wrote: On 08.08.2016 10:30, Martin Basti wrote: On 02.08.2016 14:50, Lenka Doudova wrote: On 07/29/2016 11:43 AM, Lenka Doudova wrote: On 07/29/2016 11:41 AM, Lenka Doudova wrote: On 07/28/2016 01:35 PM, Peter Lacko wrote: Hops, fixed. Peter - Original Message - From: "Lenka Doudova" To:freeipa-devel@redhat.com Sent: Thursday, July 28, 2016 1:32:25 PM Subject: Re: [Freeipa-devel] [PATCH 0003] Test validity of URIs in certificate Hi, I cannot find any attached patch :) Lenka On 07/28/2016 01:30 PM, Peter Lacko wrote: Attached you can find a patch adding test for URIs in generated certificate ipatests/test_xmlrpc/test_cert_plugin.py Since I'm leaving Red Hat in end of July, I won't be able to modify this patch anymore. Regards, Peter Hi, NACK. Code looks fine and works well on master branch, but patch does not apply on 4-3 and 4-2 branches. Peter left the company but claimed he can fix the patch if necessary, I'll communicate it with him or fix it myself. Lenka Oh, and forgot this one - PEP8 error: ./ipatests/test_xmlrpc/test_cert_plugin.py:191:80: E501 line too long (105 > 79 characters) Lenka Hi, since Peter has quit already, I took it upon myself to do minor fix and rebase to the patch. 1) i removed pylint disable comments from the patch, as they were unnecessary (this also solved PEP8 error) 2) I rebased the patch to be applicable for ipa-4-3 branch. Original functionality of the patch remains unchanged. Both fixed patches attached. Lenka Hello, 1) This is not needed +global sn + +result = api.Command.cert_show(sn, out=unicode(self.certfile)) you need the global statement only for write access. But sn is not assigned in this code block. 2) I prefer to use instance attributes (self.sn) instead of global variables As we figured out, pytest creates for each test new instance of class, so 2) will not fork. Please fix only 1), sorry. Martin^2 Hi, attached fixed patches for master and 4.3 branches. Lenka Martin^2 ACK Pushed to master: 019f3611c299532cd89321767b0e0e4df0d0db27 Pushed to ipa-4-3: 2a207dd637748a4c05e54755b755986fbed16d55 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0003] Test validity of URIs in certificate
On 08/10/2016 05:48 PM, Martin Basti wrote: On 08.08.2016 10:30, Martin Basti wrote: On 02.08.2016 14:50, Lenka Doudova wrote: On 07/29/2016 11:43 AM, Lenka Doudova wrote: On 07/29/2016 11:41 AM, Lenka Doudova wrote: On 07/28/2016 01:35 PM, Peter Lacko wrote: Hops, fixed. Peter - Original Message - From: "Lenka Doudova" To:freeipa-devel@redhat.com Sent: Thursday, July 28, 2016 1:32:25 PM Subject: Re: [Freeipa-devel] [PATCH 0003] Test validity of URIs in certificate Hi, I cannot find any attached patch :) Lenka On 07/28/2016 01:30 PM, Peter Lacko wrote: Attached you can find a patch adding test for URIs in generated certificate ipatests/test_xmlrpc/test_cert_plugin.py Since I'm leaving Red Hat in end of July, I won't be able to modify this patch anymore. Regards, Peter Hi, NACK. Code looks fine and works well on master branch, but patch does not apply on 4-3 and 4-2 branches. Peter left the company but claimed he can fix the patch if necessary, I'll communicate it with him or fix it myself. Lenka Oh, and forgot this one - PEP8 error: ./ipatests/test_xmlrpc/test_cert_plugin.py:191:80: E501 line too long (105 > 79 characters) Lenka Hi, since Peter has quit already, I took it upon myself to do minor fix and rebase to the patch. 1) i removed pylint disable comments from the patch, as they were unnecessary (this also solved PEP8 error) 2) I rebased the patch to be applicable for ipa-4-3 branch. Original functionality of the patch remains unchanged. Both fixed patches attached. Lenka Hello, 1) This is not needed +global sn + +result = api.Command.cert_show(sn, out=unicode(self.certfile)) you need the global statement only for write access. But sn is not assigned in this code block. 2) I prefer to use instance attributes (self.sn) instead of global variables As we figured out, pytest creates for each test new instance of class, so 2) will not fork. Please fix only 1), sorry. Martin^2 Hi, attached fixed patches for master and 4.3 branches. Lenka Martin^2 From 0c11503843cd4c69549751cbb3e2113e79f196d6 Mon Sep 17 00:00:00 2001 From: Peter Lacko Date: Fri, 15 Jul 2016 16:55:51 +0200 Subject: [PATCH] Test URIs in certificate. Test that CRL URI and OCSP URI are present and correct in generated certificate. https://fedorahosted.org/freeipa/ticket/5881 --- ipatests/test_xmlrpc/test_cert_plugin.py | 49 1 file changed, 37 insertions(+), 12 deletions(-) diff --git a/ipatests/test_xmlrpc/test_cert_plugin.py b/ipatests/test_xmlrpc/test_cert_plugin.py index a3839d0f79af7208bc2e9ce54183dec288f79ff1..d6a0bca72a272cd51e7da7728fd1a2618a27ff7a 100644 --- a/ipatests/test_xmlrpc/test_cert_plugin.py +++ b/ipatests/test_xmlrpc/test_cert_plugin.py @@ -19,24 +19,26 @@ """ Test the `ipalib/plugins/cert.py` module against a RA. """ +from __future__ import print_function import sys +import base64 +import nose import os +import pytest import shutil -from nose.tools import raises, assert_raises # pylint: disable=E0611 -from ipatests.test_xmlrpc.xmlrpc_test import XMLRPC_test, assert_attr_equal +import six +import tempfile from ipalib import api from ipalib import errors from ipalib import x509 -import tempfile -from ipapython import ipautil -import six -import nose -import base64 from ipaplatform.paths import paths +from ipapython import ipautil from ipapython.dn import DN -import pytest +from ipapython.ipautil import run +from ipatests.test_xmlrpc.xmlrpc_test import XMLRPC_test, assert_attr_equal +from nose.tools import raises, assert_raises if six.PY3: unicode = str @@ -44,6 +46,11 @@ if six.PY3: # So we can save the cert from issuance and compare it later cert = None newcert = None +sn = None + +_DOMAIN = api.env.domain +_EXP_CRL_URI = ''.join(['http://ipa-ca.', _DOMAIN, '/ipa/crl/MasterCRL.bin']) +_EXP_OCSP_URI = ''.join(['http://ipa-ca.', _DOMAIN, '/ca/ocsp']) def is_db_configured(): """ @@ -82,6 +89,8 @@ class test_cert(XMLRPC_test): if 'cert_request' not in api.Command: raise nose.SkipTest('cert_request not registered') +if 'cert_show' not in api.Command: +raise nose.SkipTest('cert_show not registered') is_db_configured() @@ -94,6 +103,7 @@ class test_cert(XMLRPC_test): self.reqdir = tempfile.mkdtemp(prefix = "tmp-") self.reqfile = self.reqdir + "/test.csr" self.pwname = self.reqdir + "/pwd" +self.certfile = self.reqdir + "/cert.crt" # Create an empty password file fp = open(self.pwname, "w") @@ -144,13 +154,15 @@ class test_cert(XMLRPC_test): Test the `xmlrpc.cert_request` method with --add.
Re: [Freeipa-devel] [PATCH 0003] Test validity of URIs in certificate
On 08.08.2016 10:30, Martin Basti wrote: On 02.08.2016 14:50, Lenka Doudova wrote: On 07/29/2016 11:43 AM, Lenka Doudova wrote: On 07/29/2016 11:41 AM, Lenka Doudova wrote: On 07/28/2016 01:35 PM, Peter Lacko wrote: Hops, fixed. Peter - Original Message - From: "Lenka Doudova" To:freeipa-devel@redhat.com Sent: Thursday, July 28, 2016 1:32:25 PM Subject: Re: [Freeipa-devel] [PATCH 0003] Test validity of URIs in certificate Hi, I cannot find any attached patch :) Lenka On 07/28/2016 01:30 PM, Peter Lacko wrote: Attached you can find a patch adding test for URIs in generated certificate ipatests/test_xmlrpc/test_cert_plugin.py Since I'm leaving Red Hat in end of July, I won't be able to modify this patch anymore. Regards, Peter Hi, NACK. Code looks fine and works well on master branch, but patch does not apply on 4-3 and 4-2 branches. Peter left the company but claimed he can fix the patch if necessary, I'll communicate it with him or fix it myself. Lenka Oh, and forgot this one - PEP8 error: ./ipatests/test_xmlrpc/test_cert_plugin.py:191:80: E501 line too long (105 > 79 characters) Lenka Hi, since Peter has quit already, I took it upon myself to do minor fix and rebase to the patch. 1) i removed pylint disable comments from the patch, as they were unnecessary (this also solved PEP8 error) 2) I rebased the patch to be applicable for ipa-4-3 branch. Original functionality of the patch remains unchanged. Both fixed patches attached. Lenka Hello, 1) This is not needed +global sn + +result = api.Command.cert_show(sn, out=unicode(self.certfile)) you need the global statement only for write access. But sn is not assigned in this code block. 2) I prefer to use instance attributes (self.sn) instead of global variables As we figured out, pytest creates for each test new instance of class, so 2) will not fork. Please fix only 1), sorry. Martin^2 Martin^2 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0003] Test validity of URIs in certificate
On 02.08.2016 14:50, Lenka Doudova wrote: On 07/29/2016 11:43 AM, Lenka Doudova wrote: On 07/29/2016 11:41 AM, Lenka Doudova wrote: On 07/28/2016 01:35 PM, Peter Lacko wrote: Hops, fixed. Peter - Original Message - From: "Lenka Doudova" To:freeipa-devel@redhat.com Sent: Thursday, July 28, 2016 1:32:25 PM Subject: Re: [Freeipa-devel] [PATCH 0003] Test validity of URIs in certificate Hi, I cannot find any attached patch :) Lenka On 07/28/2016 01:30 PM, Peter Lacko wrote: Attached you can find a patch adding test for URIs in generated certificate ipatests/test_xmlrpc/test_cert_plugin.py Since I'm leaving Red Hat in end of July, I won't be able to modify this patch anymore. Regards, Peter Hi, NACK. Code looks fine and works well on master branch, but patch does not apply on 4-3 and 4-2 branches. Peter left the company but claimed he can fix the patch if necessary, I'll communicate it with him or fix it myself. Lenka Oh, and forgot this one - PEP8 error: ./ipatests/test_xmlrpc/test_cert_plugin.py:191:80: E501 line too long (105 > 79 characters) Lenka Hi, since Peter has quit already, I took it upon myself to do minor fix and rebase to the patch. 1) i removed pylint disable comments from the patch, as they were unnecessary (this also solved PEP8 error) 2) I rebased the patch to be applicable for ipa-4-3 branch. Original functionality of the patch remains unchanged. Both fixed patches attached. Lenka Hello, 1) This is not needed +global sn + +result = api.Command.cert_show(sn, out=unicode(self.certfile)) you need the global statement only for write access. But sn is not assigned in this code block. 2) I prefer to use instance attributes (self.sn) instead of global variables Martin^2 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0003] Test validity of URIs in certificate
On 07/29/2016 11:43 AM, Lenka Doudova wrote: On 07/29/2016 11:41 AM, Lenka Doudova wrote: On 07/28/2016 01:35 PM, Peter Lacko wrote: Hops, fixed. Peter - Original Message - From: "Lenka Doudova" To:freeipa-devel@redhat.com Sent: Thursday, July 28, 2016 1:32:25 PM Subject: Re: [Freeipa-devel] [PATCH 0003] Test validity of URIs in certificate Hi, I cannot find any attached patch :) Lenka On 07/28/2016 01:30 PM, Peter Lacko wrote: Attached you can find a patch adding test for URIs in generated certificate ipatests/test_xmlrpc/test_cert_plugin.py Since I'm leaving Red Hat in end of July, I won't be able to modify this patch anymore. Regards, Peter Hi, NACK. Code looks fine and works well on master branch, but patch does not apply on 4-3 and 4-2 branches. Peter left the company but claimed he can fix the patch if necessary, I'll communicate it with him or fix it myself. Lenka Oh, and forgot this one - PEP8 error: ./ipatests/test_xmlrpc/test_cert_plugin.py:191:80: E501 line too long (105 > 79 characters) Lenka Hi, since Peter has quit already, I took it upon myself to do minor fix and rebase to the patch. 1) i removed pylint disable comments from the patch, as they were unnecessary (this also solved PEP8 error) 2) I rebased the patch to be applicable for ipa-4-3 branch. Original functionality of the patch remains unchanged. Both fixed patches attached. Lenka From 63f0efeaf16cff8cb30e6e8e7903722330ae883c Mon Sep 17 00:00:00 2001 From: Peter Lacko Date: Fri, 15 Jul 2016 16:55:51 +0200 Subject: [PATCH] Test URIs in certificate. Test that CRL URI and OCSP URI are present and correct in generated certificate. https://fedorahosted.org/freeipa/ticket/5881 --- ipatests/test_xmlrpc/test_cert_plugin.py | 52 1 file changed, 39 insertions(+), 13 deletions(-) diff --git a/ipatests/test_xmlrpc/test_cert_plugin.py b/ipatests/test_xmlrpc/test_cert_plugin.py index a3839d0f79af7208bc2e9ce54183dec288f79ff1..b106c091edde68097c35907ea834b37b64407735 100644 --- a/ipatests/test_xmlrpc/test_cert_plugin.py +++ b/ipatests/test_xmlrpc/test_cert_plugin.py @@ -19,24 +19,25 @@ """ Test the `ipalib/plugins/cert.py` module against a RA. """ +from __future__ import print_function import sys +import base64 +import nose import os +import pytest import shutil -from nose.tools import raises, assert_raises # pylint: disable=E0611 - -from ipatests.test_xmlrpc.xmlrpc_test import XMLRPC_test, assert_attr_equal +import six +import tempfile from ipalib import api from ipalib import errors from ipalib import x509 -import tempfile -from ipapython import ipautil -import six -import nose -import base64 from ipaplatform.paths import paths +from ipapython import ipautil from ipapython.dn import DN -import pytest +from ipapython.ipautil import run +from ipatests.test_xmlrpc.xmlrpc_test import XMLRPC_test, assert_attr_equal +from nose.tools import raises, assert_raises # pylint: disable=E0611 if six.PY3: unicode = str @@ -44,6 +45,11 @@ if six.PY3: # So we can save the cert from issuance and compare it later cert = None newcert = None +sn = None + +_DOMAIN = api.env.domain +_EXP_CRL_URI = ''.join(['http://ipa-ca.', _DOMAIN, '/ipa/crl/MasterCRL.bin']) +_EXP_OCSP_URI = ''.join(['http://ipa-ca.', _DOMAIN, '/ca/ocsp']) def is_db_configured(): """ @@ -82,6 +88,8 @@ class test_cert(XMLRPC_test): if 'cert_request' not in api.Command: raise nose.SkipTest('cert_request not registered') +if 'cert_show' not in api.Command: +raise nose.SkipTest('cert_show not registered') is_db_configured() @@ -94,6 +102,7 @@ class test_cert(XMLRPC_test): self.reqdir = tempfile.mkdtemp(prefix = "tmp-") self.reqfile = self.reqdir + "/test.csr" self.pwname = self.reqdir + "/pwd" +self.certfile = self.reqdir + "/cert.crt" # Create an empty password file fp = open(self.pwname, "w") @@ -144,13 +153,15 @@ class test_cert(XMLRPC_test): Test the `xmlrpc.cert_request` method with --add. """ # Our host should exist from previous test -global cert +global cert, sn csr = unicode(self.generateCSR(str(self.subject))) res = api.Command['cert_request'](csr, principal=self.service_princ, add=True)['result'] assert DN(res['subject']) == self.subject # save the cert for the service_show/find tests cert = res['certificate'].encode('ascii') +# save cert's SN for URI test +sn = res['serial_number'] def test_0003_service_show(self): ""&qu
Re: [Freeipa-devel] [PATCH 0003] Test validity of URIs in certificate
On 07/29/2016 11:41 AM, Lenka Doudova wrote: On 07/28/2016 01:35 PM, Peter Lacko wrote: Hops, fixed. Peter - Original Message - From: "Lenka Doudova" To:freeipa-devel@redhat.com Sent: Thursday, July 28, 2016 1:32:25 PM Subject: Re: [Freeipa-devel] [PATCH 0003] Test validity of URIs in certificate Hi, I cannot find any attached patch :) Lenka On 07/28/2016 01:30 PM, Peter Lacko wrote: Attached you can find a patch adding test for URIs in generated certificate ipatests/test_xmlrpc/test_cert_plugin.py Since I'm leaving Red Hat in end of July, I won't be able to modify this patch anymore. Regards, Peter Hi, NACK. Code looks fine and works well on master branch, but patch does not apply on 4-3 and 4-2 branches. Peter left the company but claimed he can fix the patch if necessary, I'll communicate it with him or fix it myself. Lenka Oh, and forgot this one - PEP8 error: ./ipatests/test_xmlrpc/test_cert_plugin.py:191:80: E501 line too long (105 > 79 characters) Lenka -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0003] Test validity of URIs in certificate
On 07/28/2016 01:35 PM, Peter Lacko wrote: Hops, fixed. Peter - Original Message - From: "Lenka Doudova" To: freeipa-devel@redhat.com Sent: Thursday, July 28, 2016 1:32:25 PM Subject: Re: [Freeipa-devel] [PATCH 0003] Test validity of URIs in certificate Hi, I cannot find any attached patch :) Lenka On 07/28/2016 01:30 PM, Peter Lacko wrote: Attached you can find a patch adding test for URIs in generated certificate ipatests/test_xmlrpc/test_cert_plugin.py Since I'm leaving Red Hat in end of July, I won't be able to modify this patch anymore. Regards, Peter Hi, NACK. Code looks fine and works well on master branch, but patch does not apply on 4-3 and 4-2 branches. Peter left the company but claimed he can fix the patch if necessary, I'll communicate it with him or fix it myself. Lenka -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0003] Test validity of URIs in certificate
Hops, fixed. Peter - Original Message - From: "Lenka Doudova" To: freeipa-devel@redhat.com Sent: Thursday, July 28, 2016 1:32:25 PM Subject: Re: [Freeipa-devel] [PATCH 0003] Test validity of URIs in certificate Hi, I cannot find any attached patch :) Lenka On 07/28/2016 01:30 PM, Peter Lacko wrote: > Attached you can find a patch adding test for URIs in generated certificate > ipatests/test_xmlrpc/test_cert_plugin.py > Since I'm leaving Red Hat in end of July, I won't be able to modify this > patch anymore. > > Regards, > > Peter > -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code From ce37fdab03aa3e2147578a4afb2b7087c912daaa Mon Sep 17 00:00:00 2001 From: Peter Lacko Date: Fri, 15 Jul 2016 16:55:51 +0200 Subject: [PATCH] Test URIs in certificate. Test that CRL URI and OCSP URI are present and correct in generated certificate. https://fedorahosted.org/freeipa/ticket/5881 --- ipatests/test_xmlrpc/test_cert_plugin.py | 52 1 file changed, 39 insertions(+), 13 deletions(-) diff --git a/ipatests/test_xmlrpc/test_cert_plugin.py b/ipatests/test_xmlrpc/test_cert_plugin.py index 8127ef224b24a0b3a63c3d07ef72d4b53feda4be..7688a87594f72bfdc04607965b3706e9369106aa 100644 --- a/ipatests/test_xmlrpc/test_cert_plugin.py +++ b/ipatests/test_xmlrpc/test_cert_plugin.py @@ -19,23 +19,24 @@ """ Test the `ipaserver/plugins/cert.py` module against a RA. """ +from __future__ import print_function +import base64 +import nose import os +import pytest import shutil -from nose.tools import raises, assert_raises # pylint: disable=E0611 - -from ipatests.test_xmlrpc.xmlrpc_test import XMLRPC_test +import six +import tempfile from ipalib import api from ipalib import errors from ipalib import x509 -import tempfile -from ipapython import ipautil -import six -import nose -import base64 from ipaplatform.paths import paths +from ipapython import ipautil from ipapython.dn import DN -import pytest +from ipapython.ipautil import run +from ipatests.test_xmlrpc.xmlrpc_test import XMLRPC_test +from nose.tools import raises, assert_raises # pylint: disable=E0611 if six.PY3: unicode = str @@ -43,6 +44,11 @@ if six.PY3: # So we can save the cert from issuance and compare it later cert = None newcert = None +sn = None + +_DOMAIN = api.env.domain +_EXP_CRL_URI = ''.join(['http://ipa-ca.', _DOMAIN, '/ipa/crl/MasterCRL.bin']) +_EXP_OCSP_URI = ''.join(['http://ipa-ca.', _DOMAIN, '/ca/ocsp']) def is_db_configured(): """ @@ -81,6 +87,8 @@ class test_cert(XMLRPC_test): if 'cert_request' not in api.Command: raise nose.SkipTest('cert_request not registered') +if 'cert_show' not in api.Command: +raise nose.SkipTest('cert_show not registered') is_db_configured() @@ -93,6 +101,7 @@ class test_cert(XMLRPC_test): self.reqdir = tempfile.mkdtemp(prefix = "tmp-") self.reqfile = self.reqdir + "/test.csr" self.pwname = self.reqdir + "/pwd" +self.certfile = self.reqdir + "/cert.crt" # Create an empty password file fp = open(self.pwname, "w") @@ -143,13 +152,15 @@ class test_cert(XMLRPC_test): Test the `xmlrpc.cert_request` method with --add. """ # Our host should exist from previous test -global cert +global cert, sn csr = unicode(self.generateCSR(str(self.subject))) res = api.Command['cert_request'](csr, principal=self.service_princ, add=True)['result'] assert DN(res['subject']) == self.subject # save the cert for the service_show/find tests cert = res['certificate'].encode('ascii') +# save cert's SN for URI test +sn = res['serial_number'] def test_0003_service_show(self): """ @@ -170,7 +181,22 @@ class test_cert(XMLRPC_test): res = api.Command['service_find'](self.service_princ)['result'] assert base64.b64encode(res[0]['usercertificate'][0]) == cert -def test_0005_cert_renew(self): +def test_0005_cert_uris(self): +"""Test URI details and OCSP-URI in certificate. + +See https://fedorahosted.org/freeipa/ticket/5881 +""" +global sn + +result = api.Command.cert_show(sn, out=unicode(self.certfile)) # pylint: disable=unicode-builtin +with open(self.certfile, "r") as f: +pem_cert = unicode(f.read()) # pylint: disable=unicode
Re: [Freeipa-devel] [PATCH 0003] Test validity of URIs in certificate
Hi, I cannot find any attached patch :) Lenka On 07/28/2016 01:30 PM, Peter Lacko wrote: Attached you can find a patch adding test for URIs in generated certificate ipatests/test_xmlrpc/test_cert_plugin.py Since I'm leaving Red Hat in end of July, I won't be able to modify this patch anymore. Regards, Peter -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code