openlp-core team mailing list archive
-
openlp-core team
-
Mailing list archive
-
Message #29951
[Merge] lp:~alisonken1/openlp/bug-1593883-projector-authentication into lp:openlp
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))
Follow ups