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"<ldoud...@redhat.com>
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 <pla...@redhat.com>
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.
         """
         # 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):
         """
@@ -171,7 +183,20 @@ 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
+        """
+        result = api.Command.cert_show(sn, out=unicode(self.certfile))
+        with open(self.certfile, "r") as f:
+            pem_cert = unicode(f.read())
+        result = run(['openssl', 'x509', '-text'],
+                     stdin=pem_cert, capture_output=True)
+        assert _EXP_CRL_URI in result.output
+        assert _EXP_OCSP_URI in result.output
+
+    def test_0006_cert_renew(self):
         """
         Issue a new certificate for a service
         """
@@ -183,7 +208,7 @@ class test_cert(XMLRPC_test):
         # save the cert for the service_show/find tests
         newcert = res['certificate'].encode('ascii')
 
-    def test_0006_service_show(self):
+    def test_0007_service_show(self):
         """
         Verify the new certificate with service-show.
         """
@@ -195,7 +220,7 @@ class test_cert(XMLRPC_test):
         certs_encoded = (base64.b64encode(cert) for cert in res['usercertificate'])
         assert set(certs_encoded) == set([cert, newcert])
 
-    def test_0007_cleanup(self):
+    def test_0008_cleanup(self):
         """
         Clean up cert test data
         """
-- 
2.7.4

From 1ad3849b1e083138dabd2bb0d7f5531e69630402 Mon Sep 17 00:00:00 2001
From: Peter Lacko <pla...@redhat.com>
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 | 50 +++++++++++++++++++++++---------
 1 file changed, 37 insertions(+), 13 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_cert_plugin.py b/ipatests/test_xmlrpc/test_cert_plugin.py
index 8127ef224b24a0b3a63c3d07ef72d4b53feda4be..fb4ab582d6a8b8255c2fec47bf744e77f9b93aa4 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
 
 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,20 @@ 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
+        """
+        result = api.Command.cert_show(sn, out=unicode(self.certfile))
+        with open(self.certfile, "r") as f:
+            pem_cert = unicode(f.read())
+        result = run(['openssl', 'x509', '-text'],
+                     stdin=pem_cert, capture_output=True)
+        assert _EXP_CRL_URI in result.output
+        assert _EXP_OCSP_URI in result.output
+
+    def test_0006_cert_renew(self):
         """
         Issue a new certificate for a service
         """
@@ -182,7 +206,7 @@ class test_cert(XMLRPC_test):
         # save the cert for the service_show/find tests
         newcert = res['certificate'].encode('ascii')
 
-    def test_0006_service_show(self):
+    def test_0007_service_show(self):
         """
         Verify the new certificate with service-show.
         """
@@ -194,7 +218,7 @@ class test_cert(XMLRPC_test):
         certs_encoded = (base64.b64encode(cert) for cert in res['usercertificate'])
         assert set(certs_encoded) == set([cert, newcert])
 
-    def test_0007_cleanup(self):
+    def test_0008_cleanup(self):
         """
         Clean up cert test data
         """
-- 
2.7.4

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

Reply via email to