From 6c68b17b45c1e65411d13f49e4238857feb0a141 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 4 Jun 2011 19:44:08 +0200 Subject: [PATCH] Fix 'commands' and 'notcommands' for unauthenticated users Use newly gained access to the current user's authentication state and the command handler's auth_required flag to give correct 'commands' and 'notcommands' output to unauthenticated users when password authentication is activated. --- docs/changes.rst | 4 ++ mopidy/frontends/mpd/protocol/reflection.py | 43 ++++++++++++--------- tests/frontends/mpd/reflection_test.py | 28 ++++++++++++++ 3 files changed, 56 insertions(+), 19 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index 37ce22c1..b4d56711 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -40,6 +40,10 @@ No description yet. - Do not allow access to the command ``kill``. + - ``commands`` and ``notcommands`` now have correct output if password + authentication is turned on, but the connected user has not been + authenticated yet. + v0.4.1 (2011-05-06) =================== diff --git a/mopidy/frontends/mpd/protocol/reflection.py b/mopidy/frontends/mpd/protocol/reflection.py index fd47e57e..920f48a5 100644 --- a/mopidy/frontends/mpd/protocol/reflection.py +++ b/mopidy/frontends/mpd/protocol/reflection.py @@ -10,23 +10,29 @@ def commands(context): Shows which commands the current user has access to. """ - # FIXME When password auth is turned on and the client is not - # authenticated, 'commands' should list only the commands the client does - # have access to. To implement this we need access to the session object to - # check if the client is authenticated or not. - - command_names = [command.name for command in mpd_commands] + if context.dispatcher.authenticated: + command_names = [command.name for command in mpd_commands] + else: + command_names = [command.name for command in mpd_commands + if not command.auth_required] # No permission to use - command_names.remove('kill') + if 'kill' in command_names: + command_names.remove('kill') # Not shown by MPD in its command list - command_names.remove('command_list_begin') - command_names.remove('command_list_ok_begin') - command_names.remove('command_list_end') - command_names.remove('idle') - command_names.remove('noidle') - command_names.remove('sticker') + if 'command_list_begin' in command_names: + command_names.remove('command_list_begin') + if 'command_list_ok_begin' in command_names: + command_names.remove('command_list_ok_begin') + if 'command_list_end' in command_names: + command_names.remove('command_list_end') + if 'idle' in command_names: + command_names.remove('idle') + if 'noidle' in command_names: + command_names.remove('noidle') + if 'sticker' in command_names: + command_names.remove('sticker') return [('command', command_name) for command_name in sorted(command_names)] @@ -58,12 +64,11 @@ def notcommands(context): Shows which commands the current user does not have access to. """ - # FIXME When password auth is turned on and the client is not - # authenticated, 'notcommands' should list all the commands the client does - # not have access to. To implement this we need access to the session - # object to check if the client is authenticated or not. - - command_names = [] + if context.dispatcher.authenticated: + command_names = [] + else: + command_names = [command.name for command in mpd_commands + if command.auth_required] # No permission to use command_names.append('kill') diff --git a/tests/frontends/mpd/reflection_test.py b/tests/frontends/mpd/reflection_test.py index adc34338..2abf5acc 100644 --- a/tests/frontends/mpd/reflection_test.py +++ b/tests/frontends/mpd/reflection_test.py @@ -1,5 +1,6 @@ import unittest +from mopidy import settings from mopidy.backends.dummy import DummyBackend from mopidy.frontends.mpd.dispatcher import MpdDispatcher from mopidy.mixers.dummy import DummyMixer @@ -11,6 +12,7 @@ class ReflectionHandlerTest(unittest.TestCase): self.dispatcher = MpdDispatcher() def tearDown(self): + settings.runtime.clear() self.backend.stop().get() self.mixer.stop().get() @@ -31,6 +33,19 @@ class ReflectionHandlerTest(unittest.TestCase): self.assert_(u'command: sticker' not in result) self.assert_(u'OK' in result) + def test_commands_show_less_if_auth_required_and_not_authed(self): + settings.MPD_SERVER_PASSWORD = u'secret' + result = self.dispatcher.handle_request(u'commands') + # Not requiring auth + self.assert_(u'command: close' in result, result) + self.assert_(u'command: commands' in result, result) + self.assert_(u'command: notcommands' in result, result) + self.assert_(u'command: password' in result, result) + self.assert_(u'command: ping' in result, result) + # Requiring auth + self.assert_(u'command: play' not in result, result) + self.assert_(u'command: status' not in result, result) + def test_decoders(self): result = self.dispatcher.handle_request(u'decoders') self.assert_(u'ACK [0@0] {} Not implemented' in result) @@ -41,6 +56,19 @@ class ReflectionHandlerTest(unittest.TestCase): self.assert_(u'command: kill' in result) self.assert_(u'OK' in result) + def test_notcommands_returns_more_if_auth_required_and_not_authed(self): + settings.MPD_SERVER_PASSWORD = u'secret' + result = self.dispatcher.handle_request(u'notcommands') + # Not requiring auth + self.assert_(u'command: close' not in result, result) + self.assert_(u'command: commands' not in result, result) + self.assert_(u'command: notcommands' not in result, result) + self.assert_(u'command: password' not in result, result) + self.assert_(u'command: ping' not in result, result) + # Requiring auth + self.assert_(u'command: play' in result, result) + self.assert_(u'command: status' in result, result) + def test_tagtypes(self): result = self.dispatcher.handle_request(u'tagtypes') self.assert_(u'OK' in result)