[auth][api] Remove deprecated QSslCertificate::isValid()

Substitue QgsAuthCertUtils::certIsViable(cert), which checks similar characteristics to old function, plus whether the cert is null.
qgis · Oct 27, 2017 · 8032de8 · 8032de8
1 parent 88a80e6
commit 8032de8
Show file tree

Hide file tree

Showing 5 changed files with 142 additions and 15 deletions.
diff --git a/python/core/auth/qgsauthcertutils.sip b/python/core/auth/qgsauthcertutils.sip
@@ -294,6 +294,29 @@ Get short strings describing an SSL error
 %End
 
 
+    static bool certIsCurrent( const QSslCertificate &cert );
+%Docstring
+ certIsCurrent checks if ``cert`` is viable for its not before and not after dates
+ \param cert certificate to be checked
+ :rtype: bool
+%End
+
+    static QList<QSslError> certViabilityErrors( const QSslCertificate &cert );
+%Docstring
+ certViabilityErrors checks basic characteristics (validity dates, blacklisting, etc.) of given ``cert``
+ \param cert certificate to be checked
+ :return: list of QSslError (will return NO ERRORS if a null QSslCertificate is passed)
+ :rtype: list of QSslError
+%End
+
+    static bool certIsViable( const QSslCertificate &cert );
+%Docstring
+ certIsViable checks for viability errors of ``cert`` and whether it is NULL
+ \param cert certificate to be checked
+ :return: false if cert is NULL or has viability errors
+ :rtype: bool
+%End
+
     static QList<QSslError> validateCertChain( const QList<QSslCertificate> &certificateChain,
         const QString &hostName = QString(),
         bool trustRootCa = false ) ;

diff --git a/src/core/auth/qgsauthcertutils.cpp b/src/core/auth/qgsauthcertutils.cpp
@@ -1241,6 +1241,43 @@ QList<QPair<QSslError::SslError, QString> > QgsAuthCertUtils::sslErrorEnumString
   return errenums;
 }
 
+bool QgsAuthCertUtils::certIsCurrent( const QSslCertificate &cert )
+{
+  if ( cert.isNull() )
+    return false;
+  const QDateTime currentTime = QDateTime::currentDateTime();
+  return cert.effectiveDate() <= currentTime && cert.expiryDate() >= currentTime;
+}
+
+QList<QSslError> QgsAuthCertUtils::certViabilityErrors( const QSslCertificate &cert )
+{
+  QList<QSslError> sslErrors;
+
+  if ( cert.isNull() )
+    return sslErrors;
+
+  const QDateTime currentTime = QDateTime::currentDateTime();
+  if ( cert.expiryDate() <= currentTime )
+  {
+    sslErrors << QSslError( QSslError::SslError::CertificateExpired, cert );
+  }
+  if ( cert.effectiveDate() >= QDateTime::currentDateTime() )
+  {
+    sslErrors << QSslError( QSslError::SslError::CertificateNotYetValid, cert );
+  }
+  if ( cert.isBlacklisted() )
+  {
+    sslErrors << QSslError( QSslError::SslError::CertificateBlacklisted, cert );
+  }
+
+  return sslErrors;
+}
+
+bool QgsAuthCertUtils::certIsViable( const QSslCertificate &cert )
+{
+  return !cert.isNull() && QgsAuthCertUtils::certViabilityErrors( cert ).isEmpty();
+}
+
 QList<QSslError> QgsAuthCertUtils::validateCertChain( const QList<QSslCertificate> &certificateChain,
     const QString &hostName,
     bool trustRootCa )
@@ -1269,20 +1306,7 @@ QList<QSslError> QgsAuthCertUtils::validateCertChain( const QList<QSslCertificat
   const QList<QSslCertificate> constTrustedChain( trustedChain );
   for ( const auto &cert : constTrustedChain )
   {
-    // TODO: move all the checks to QgsAuthCertUtils::certIsViable( )
-    const QDateTime currentTime = QDateTime::currentDateTime();
-    if ( cert.expiryDate() <= currentTime )
-    {
-      sslErrors << QSslError( QSslError::SslError::CertificateExpired, cert );
-    }
-    if ( cert.effectiveDate() >= QDateTime::currentDateTime() )
-    {
-      sslErrors << QSslError( QSslError::SslError::CertificateNotYetValid, cert );
-    }
-    if ( cert.isBlacklisted() )
-    {
-      sslErrors << QSslError( QSslError::SslError::CertificateBlacklisted, cert );
-    }
+    sslErrors << QgsAuthCertUtils::certViabilityErrors( cert );
   }
 
   // Merge in the root CA if present and asked for

diff --git a/src/core/auth/qgsauthcertutils.h b/src/core/auth/qgsauthcertutils.h
@@ -330,6 +330,26 @@ class CORE_EXPORT QgsAuthCertUtils
      */
     static QList<QPair<QSslError::SslError, QString> > sslErrorEnumStrings() SIP_SKIP;
 
+    /**
+     * \brief certIsCurrent checks if \a cert is viable for its not before and not after dates
+     * \param cert certificate to be checked
+     */
+    static bool certIsCurrent( const QSslCertificate &cert );
+
+    /**
+     * \brief certViabilityErrors checks basic characteristics (validity dates, blacklisting, etc.) of given \a cert
+     * \param cert certificate to be checked
+     * \return list of QSslError (will return NO ERRORS if a null QSslCertificate is passed)
+     */
+    static QList<QSslError> certViabilityErrors( const QSslCertificate &cert );
+
+    /**
+     * \brief certIsViable checks for viability errors of \a cert and whether it is NULL
+     * \param cert certificate to be checked
+     * \return false if cert is NULL or has viability errors
+     */
+    static bool certIsViable( const QSslCertificate &cert );
+
     /**
      * \brief validateCertChain validates the given \a certificateChain
      * \param certificateChain list of certificates to be checked, with leaf first and with optional root CA last

diff --git a/tests/src/core/testqgsauthcertutils.cpp b/tests/src/core/testqgsauthcertutils.cpp
@@ -39,6 +39,7 @@ class TestQgsAuthCertUtils: public QObject
     void init() {}
     void cleanup() {}
 
+    void testValidationUtils();
     void testPkcsUtils();
 
   private:
@@ -60,6 +61,36 @@ void TestQgsAuthCertUtils::cleanupTestCase()
   QgsApplication::exitQgis();
 }
 
+void TestQgsAuthCertUtils::testValidationUtils()
+{
+  // null cert
+  QSslCertificate cert;
+  QVERIFY( !QgsAuthCertUtils::certIsCurrent( cert ) );
+  QList<QSslError> res = QgsAuthCertUtils::certViabilityErrors( cert );
+  QVERIFY( res.count() == 0 );
+  QVERIFY( !QgsAuthCertUtils::certIsViable( cert ) );
+
+  cert.clear();
+  res.clear();
+  // valid cert
+  cert = QgsAuthCertUtils::certFromFile( sPkiData + "/gerardus_cert.pem" );
+  QVERIFY( QgsAuthCertUtils::certIsCurrent( cert ) );
+  res = QgsAuthCertUtils::certViabilityErrors( cert );
+  QVERIFY( res.count() == 0 );
+  QVERIFY( QgsAuthCertUtils::certIsViable( cert ) );
+
+
+  cert.clear();
+  res.clear();
+  // expired cert
+  cert = QgsAuthCertUtils::certFromFile( sPkiData + "/marinus_cert-EXPIRED.pem" );
+  QVERIFY( !QgsAuthCertUtils::certIsCurrent( cert ) );
+  res = QgsAuthCertUtils::certViabilityErrors( cert );
+  QVERIFY( res.count() > 0 );
+  QVERIFY( res.contains( QSslError( QSslError::SslError::CertificateExpired, cert ) ) );
+  QVERIFY( !QgsAuthCertUtils::certIsViable( cert ) );
+}
+
 void TestQgsAuthCertUtils::testPkcsUtils()
 {
   QByteArray pkcs;

diff --git a/tests/src/python/test_qgsauthsystem.py b/tests/src/python/test_qgsauthsystem.py
@@ -20,7 +20,7 @@
 from qgis.core import QgsAuthCertUtils, QgsPkiBundle, QgsAuthMethodConfig, QgsAuthMethod, QgsAuthConfigSslServer, QgsApplication
 from qgis.gui import QgsAuthEditorWidgets
 from qgis.PyQt.QtCore import QFileInfo, qDebug
-from qgis.PyQt.QtNetwork import QSsl, QSslError, QSslSocket
+from qgis.PyQt.QtNetwork import QSsl, QSslError, QSslCertificate, QSslSocket
 from qgis.PyQt.QtTest import QTest
 from qgis.PyQt.QtWidgets import QDialog, QDialogButtonBox, QVBoxLayout
 from qgis.testing import start_app, unittest
@@ -714,6 +714,35 @@ def mkPEMBundle(client_cert, client_key, password, chain):
         # Test valid with intermediates and untrusted root
         self.assertEqual(QgsAuthCertUtils.validatePKIBundle(bundle, True, True), ['The issuer certificate of a locally looked up certificate could not be found'])
 
+    def test_160_cert_viable(self):
+        """Text the viability of a given certificate"""
+
+        # null cert
+        cert = QSslCertificate()
+        self.assertFalse(QgsAuthCertUtils.certIsCurrent(cert))
+        res = QgsAuthCertUtils.certViabilityErrors(cert)
+        self.assertTrue(len(res) == 0)
+        self.assertFalse(QgsAuthCertUtils.certIsViable(cert))
+
+        cert.clear()
+        res.clear()
+        # valid cert
+        cert = QgsAuthCertUtils.certFromFile(PKIDATA + '/gerardus_cert.pem')
+        self.assertTrue(QgsAuthCertUtils.certIsCurrent(cert))
+        res = QgsAuthCertUtils.certViabilityErrors(cert)
+        self.assertTrue(len(res) == 0)
+        self.assertTrue(QgsAuthCertUtils.certIsViable(cert))
+
+        cert.clear()
+        res.clear()
+        # expired cert
+        cert = QgsAuthCertUtils.certFromFile(PKIDATA + '/marinus_cert-EXPIRED.pem')
+        self.assertFalse(QgsAuthCertUtils.certIsCurrent(cert))
+        res = QgsAuthCertUtils.certViabilityErrors(cert)
+        self.assertTrue(len(res) > 0)
+        self.assertTrue(QSslError(QSslError.CertificateExpired, cert) in res)
+        self.assertFalse(QgsAuthCertUtils.certIsViable(cert))
+
 
 if __name__ == '__main__':
     unittest.main()