openlp-core team mailing list archive

[Ken Roberts <alisonken1@xxxxxxxxx>]

Ken Roberts has proposed merging lp:~alisonken1/openlp/bug-1593883-projector-authentication into lp:openlp.

Commit message:
Bugfix 1593882 and 1593883 - projector authorization

Requested reviews:
  OpenLP Core (openlp-core)
Related bugs:
  Bug #1593882 in OpenLP: "Projector authenticated connection generates exception"
  https://bugs.launchpad.net/openlp/+bug/1593882
  Bug #1593883 in OpenLP: "Projector not connecting with authentication"
  https://bugs.launchpad.net/openlp/+bug/1593883

For more details, see:
https://code.launchpad.net/~alisonken1/openlp/bug-1593883-projector-authentication/+merge/297819

Bugfix 1593882 and 1593883 - projector authorization

- Fix exception when authenticated connection requested and pin is None
- Fix pjlink authentication (use python hash instead of qt hash)
- Fix md5_hash functions
- Fix qmd5hash functions
- Added tests for bugfixes
Tested with test server and Eiki XL200 projector

--------------------------------
lp:~alisonken1/openlp/bug-1593883-projector-authentication (revision 2684)
[SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/1629/
[SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/1540/
[SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1478/
[SUCCESS] https://ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/1247/
[SUCCESS] https://ci.openlp.io/job/Branch-04b-Windows_Interface_Tests/837/
[SUCCESS] https://ci.openlp.io/job/Branch-05a-Code_Analysis/905/
[SUCCESS] https://ci.openlp.io/job/Branch-05b-Test_Coverage/773/

-- 
Your team OpenLP Core is requested to review the proposed merge of lp:~alisonken1/openlp/bug-1593883-projector-authentication into lp:openlp.

=== modified file 'openlp/core/common/__init__.py'
--- openlp/core/common/__init__.py	2016-05-05 19:37:48 +0000
+++ openlp/core/common/__init__.py	2016-06-18 02:53:34 +0000
@@ -226,10 +226,11 @@
     log.debug('qmd5_hash(salt="{text}"'.format(text=salt))
     hash_obj = QHash(QHash.Md5)
     hash_obj.addData(salt)
-    hash_obj.addData(data)
+    if data:
+        hash_obj.addData(data)
     hash_value = hash_obj.result().toHex()
-    log.debug('qmd5_hash() returning "{text}"'.format(text=hash_value))
-    return hash_value.data()
+    log.debug('qmd5_hash() returning "{hash}"'.format(hash=hash_value))
+    return hash_value
 
 
 def clean_button_text(button_text):

=== modified file 'openlp/core/lib/projector/pjlink1.py'
--- openlp/core/lib/projector/pjlink1.py	2016-05-28 05:50:31 +0000
+++ openlp/core/lib/projector/pjlink1.py	2016-06-18 02:53:34 +0000
@@ -49,7 +49,7 @@
 from PyQt5.QtCore import pyqtSignal, pyqtSlot
 from PyQt5.QtNetwork import QAbstractSocket, QTcpSocket
 
-from openlp.core.common import translate, qmd5_hash
+from openlp.core.common import translate, md5_hash
 from openlp.core.lib.projector.constants import *
 
 # Shortcuts
@@ -297,6 +297,8 @@
         Processes the initial connection and authentication (if needed).
         Starts poll timer if connection is established.
 
+        NOTE: Qt md5 hash function doesn't work with projector authentication. Use the python md5 hash function.
+
         :param data: Optional data if called from another routine
         """
         log.debug('({ip}) check_login(data="{data}")'.format(ip=self.ip, data=data))
@@ -344,16 +346,25 @@
             return
         elif data_check[1] == '0' and self.pin is not None:
             # Pin set and no authentication needed
+            log.warning('({ip}) Regular connection but PIN set'.format(ip=self.name))
             self.disconnect_from_host()
             self.change_status(E_AUTHENTICATION)
-            log.debug('({ip}) emitting projectorNoAuthentication() signal'.format(ip=self.name))
+            log.debug('({ip}) Emitting projectorNoAuthentication() signal'.format(ip=self.name))
             self.projectorNoAuthentication.emit(self.name)
             return
         elif data_check[1] == '1':
             # Authenticated login with salt
-            log.debug('({ip}) Setting hash with salt="{data}"'.format(ip=self.ip, data=data_check[2]))
-            log.debug('({ip}) pin="{data}"'.format(ip=self.ip, data=self.pin))
-            salt = qmd5_hash(salt=data_check[2].encode('ascii'), data=self.pin.encode('ascii'))
+            if self.pin is None:
+                log.warning('({ip}) Authenticated connection but no pin set'.format(ip=self.name))
+                self.disconnect_from_host()
+                self.change_status(E_AUTHENTICATION)
+                log.debug('({ip}) Emitting projectorAuthentication() signal'.format(ip=self.name))
+                self.projectorAuthentication.emit(self.name)
+                return
+            else:
+                log.debug('({ip}) Setting hash with salt="{data}"'.format(ip=self.ip, data=data_check[2]))
+                log.debug('({ip}) pin="{data}"'.format(ip=self.ip, data=self.pin))
+                salt = md5_hash(salt=data_check[2].encode('ascii'), data=self.pin.encode('ascii'))
         else:
             salt = None
         # We're connected at this point, so go ahead and do regular I/O

=== modified file 'tests/functional/openlp_core_common/test_projector_utilities.py'
--- tests/functional/openlp_core_common/test_projector_utilities.py	2016-04-17 18:48:50 +0000
+++ tests/functional/openlp_core_common/test_projector_utilities.py	2016-06-18 02:53:34 +0000
@@ -147,7 +147,7 @@
         hash_ = qmd5_hash(salt=salt.encode('ascii'), data=pin.encode('ascii'))
 
         # THEN: Validate return has is same
-        self.assertEquals(hash_.decode('ascii'), test_hash, 'Qt-MD5 should have returned a good hash')
+        self.assertEquals(hash_, test_hash, 'Qt-MD5 should have returned a good hash')
 
     def test_qmd5_hash_bad(self):
         """
@@ -157,7 +157,7 @@
         hash_ = qmd5_hash(salt=pin.encode('ascii'), data=salt.encode('ascii'))
 
         # THEN: return data is different
-        self.assertNotEquals(hash_.decode('ascii'), test_hash, 'Qt-MD5 should have returned a bad hash')
+        self.assertNotEquals(hash_, test_hash, 'Qt-MD5 should have returned a bad hash')
 
     def test_md5_non_ascii_string(self):
         """

=== modified file 'tests/functional/openlp_core_lib/test_projector_pjlink1.py'
--- tests/functional/openlp_core_lib/test_projector_pjlink1.py	2016-05-31 21:40:13 +0000
+++ tests/functional/openlp_core_lib/test_projector_pjlink1.py	2016-06-18 02:53:34 +0000
@@ -30,7 +30,7 @@
     S_COOLDOWN, PJLINK_POWR_STATUS
 
 from tests.functional import patch
-from tests.resources.projector.data import TEST_PIN, TEST_SALT, TEST_CONNECT_AUTHENTICATE
+from tests.resources.projector.data import TEST_PIN, TEST_SALT, TEST_CONNECT_AUTHENTICATE, TEST_HASH
 
 pjlink_test = PJLink1(name='test', ip='127.0.0.1', pin=TEST_PIN, no_poll=True)
 
@@ -332,3 +332,53 @@
         self.assertFalse(pjlink.send_busy, 'Projector send_busy should be False')
         self.assertTrue(mock_timer.called, 'Projector timer.stop()  should have been called')
         self.assertTrue(mock_socket_timer.called, 'Projector socket_timer.stop() should have been called')
+
+    @patch.object(pjlink_test, 'send_command')
+    @patch.object(pjlink_test, 'waitForReadyRead')
+    @patch.object(pjlink_test, 'projectorAuthentication')
+    @patch.object(pjlink_test, 'timer')
+    @patch.object(pjlink_test, 'socket_timer')
+    def test_bug_1593882_no_pin_authenticated_connection(self, mock_socket_timer,
+                                                         mock_timer,
+                                                         mock_authentication,
+                                                         mock_ready_read,
+                                                         mock_send_command):
+        """
+        Test bug 1593882 no pin and authenticated request exception
+        """
+        # GIVEN: Test object and mocks
+        pjlink = pjlink_test
+        pjlink.pin = None
+        mock_ready_read.return_value = True
+
+        # WHEN: call with authentication request and pin not set
+        pjlink.check_login(data=TEST_CONNECT_AUTHENTICATE)
+
+        # THEN: No Authentication signal should have been sent
+        mock_authentication.emit.assert_called_with(pjlink.name)
+
+    @patch.object(pjlink_test, 'waitForReadyRead')
+    @patch.object(pjlink_test, 'state')
+    @patch.object(pjlink_test, '_send_command')
+    @patch.object(pjlink_test, 'timer')
+    @patch.object(pjlink_test, 'socket_timer')
+    def test_bug_1593883_pjlink_authentication(self, mock_socket_timer,
+                                               mock_timer,
+                                               mock_send_command,
+                                               mock_state,
+                                               mock_waitForReadyRead):
+        """
+        Test bugfix 1593883 pjlink authentication
+        """
+        # GIVEN: Test object and data
+        pjlink = pjlink_test
+        pjlink.pin = TEST_PIN
+        mock_state.return_value = pjlink.ConnectedState
+        mock_waitForReadyRead.return_value = True
+
+        # WHEN: Athenticated connection is called
+        pjlink.check_login(data=TEST_CONNECT_AUTHENTICATE)
+
+        # THEN: send_command should have the proper authentication
+        self.assertEquals("{test}".format(test=mock_send_command.call_args),
+                          "call(data='{hash}%1CLSS ?\\r')".format(hash=TEST_HASH))