Launchpad itself

Merge lp:~jml/launchpad/poppy-cleanup into lp:launchpad

  1. poppy-cleanup
  2. Merge into devel
Proposed by Jonathan Lange
Status: Merged
Approved by: Brad Crittenden
Approved revision: no longer in the source branch.
Merged at revision: 11071
Proposed branch: lp:~jml/launchpad/poppy-cleanup
Merge into: lp:launchpad
Diff against target: 270 lines (+155/-26)
4 files modified
daemons/poppy-sftp.tac (+4/-0)
lib/lp/codehosting/sshserver/session.py (+3/-25)
lib/lp/services/sshserver/session.py (+43/-1)
lib/lp/services/sshserver/tests/test_session.py (+105/-0)
To merge this branch: bzr merge lp:~jml/launchpad/poppy-cleanup
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code Approve
Review via email: mp+28525@code.launchpad.net

Description of the change

This branch fixes bug 598532 and a related, unreported bug about poppy giving an ugly error when you try to execute a command on the server (e.g. "ssh ppa.launchpad.net ls")

It does so by providing a "session" adapter for poppy that provides no services itself. The session adapter is a user's view of the SSH server (minus any apps like sftp). For poppy, we don't want shells, terminals or executed processes, so a "do nothing" adapter is the right fit.

The code was written by stealing the relevant bits from the codehosting session adapter, which does pratically nothing.

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :

Nice branch Jono

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'daemons/poppy-sftp.tac'
--- daemons/poppy-sftp.tac 2010-05-27 07:07:24 +0000
+++ daemons/poppy-sftp.tac 2010-06-25 17:21:32 +0000
@@ -8,6 +8,7 @@
8import os8import os
99
10from twisted.application import service10from twisted.application import service
11from twisted.conch.interfaces import ISession
11from twisted.conch.ssh import filetransfer12from twisted.conch.ssh import filetransfer
12from twisted.cred.portal import IRealm, Portal13from twisted.cred.portal import IRealm, Portal
13from twisted.python import components14from twisted.python import components
@@ -22,6 +23,7 @@
22from lp.services.sshserver.auth import (23from lp.services.sshserver.auth import (
23 LaunchpadAvatar, PublicKeyFromLaunchpadChecker)24 LaunchpadAvatar, PublicKeyFromLaunchpadChecker)
24from lp.services.sshserver.service import SSHService25from lp.services.sshserver.service import SSHService
26from lp.services.sshserver.session import DoNothingSession
2527
26# XXX: Rename this file to something that doesn't mention poppy. Talk to28# XXX: Rename this file to something that doesn't mention poppy. Talk to
27# bigjools.29# bigjools.
@@ -79,6 +81,8 @@
79components.registerAdapter(81components.registerAdapter(
80 poppy_sftp_adapter, LaunchpadAvatar, filetransfer.ISFTPServer)82 poppy_sftp_adapter, LaunchpadAvatar, filetransfer.ISFTPServer)
8183
84components.registerAdapter(DoNothingSession, LaunchpadAvatar, ISession)
85
8286
83# Construct an Application that has the Poppy SSH server.87# Construct an Application that has the Poppy SSH server.
84application = service.Application('poppy-sftp')88application = service.Application('poppy-sftp')
8589
=== modified file 'lib/lp/codehosting/sshserver/session.py'
--- lib/lp/codehosting/sshserver/session.py 2010-04-15 15:41:05 +0000
+++ lib/lp/codehosting/sshserver/session.py 2010-06-25 17:21:32 +0000
@@ -12,16 +12,14 @@
12import urlparse12import urlparse
1313
14from zope.event import notify14from zope.event import notify
15from zope.interface import implements
1615
17from twisted.conch.interfaces import ISession
18from twisted.conch.ssh import connection
19from twisted.internet.process import ProcessExitedAlready16from twisted.internet.process import ProcessExitedAlready
20from twisted.python import log17from twisted.python import log
2118
22from canonical.config import config19from canonical.config import config
23from lp.codehosting import get_bzr_path20from lp.codehosting import get_bzr_path
24from lp.services.sshserver.events import AvatarEvent21from lp.services.sshserver.events import AvatarEvent
22from lp.services.sshserver.session import DoNothingSession
2523
2624
27class BazaarSSHStarted(AvatarEvent):25class BazaarSSHStarted(AvatarEvent):
@@ -38,13 +36,11 @@
38 """Raised when a session is asked to execute a forbidden command."""36 """Raised when a session is asked to execute a forbidden command."""
3937
4038
41class ExecOnlySession:39class ExecOnlySession(DoNothingSession):
42 """Conch session that only allows executing commands."""40 """Conch session that only allows executing commands."""
4341
44 implements(ISession)
45
46 def __init__(self, avatar, reactor, environment=None):42 def __init__(self, avatar, reactor, environment=None):
47 self.avatar = avatar43 super(ExecOnlySession, self).__init__(avatar)
48 self.reactor = reactor44 self.reactor = reactor
49 self.environment = environment45 self.environment = environment
50 self._transport = None46 self._transport = None
@@ -72,11 +68,6 @@
72 if self._transport is not None:68 if self._transport is not None:
73 self._transport.closeStdin()69 self._transport.closeStdin()
7470
75 def errorWithMessage(self, protocol, msg):
76 protocol.session.writeExtended(
77 connection.EXTENDED_DATA_STDERR, msg)
78 protocol.loseConnection()
79
80 def execCommand(self, protocol, command):71 def execCommand(self, protocol, command):
81 """Executes `command` using `protocol` as the ProcessProtocol.72 """Executes `command` using `protocol` as the ProcessProtocol.
8273
@@ -115,19 +106,6 @@
115 args = command.split()106 args = command.split()
116 return args[0], args107 return args[0], args
117108
118 def getPty(self, term, windowSize, modes):
119 """See ISession."""
120 # Do nothing, as we don't provide shell access. openShell will get
121 # called and handle this error message and disconnect.
122
123 def openShell(self, protocol):
124 """See ISession."""
125 self.errorWithMessage(protocol, "No shells on this server.\r\n")
126
127 def windowChanged(self, newWindowSize):
128 """See ISession."""
129 raise NotImplementedError()
130
131109
132class RestrictedExecOnlySession(ExecOnlySession):110class RestrictedExecOnlySession(ExecOnlySession):
133 """Conch session that only allows a single command to be executed."""111 """Conch session that only allows a single command to be executed."""
134112
=== modified file 'lib/lp/services/sshserver/session.py'
--- lib/lp/services/sshserver/session.py 2010-04-15 15:32:16 +0000
+++ lib/lp/services/sshserver/session.py 2010-06-25 17:21:32 +0000
@@ -5,10 +5,14 @@
55
6__metaclass__ = type6__metaclass__ = type
7__all__ = [7__all__ = [
8 'DoNothingSession',
8 'PatchedSSHSession',9 'PatchedSSHSession',
9 ]10 ]
1011
11from twisted.conch.ssh import channel, session12from zope.interface import implements
13
14from twisted.conch.interfaces import ISession
15from twisted.conch.ssh import channel, connection, session
1216
1317
14class PatchedSSHSession(session.SSHSession, object):18class PatchedSSHSession(session.SSHSession, object):
@@ -77,3 +81,41 @@
77 resumeProducing = getattr(transport, 'resumeProducing', None)81 resumeProducing = getattr(transport, 'resumeProducing', None)
78 if resumeProducing is not None:82 if resumeProducing is not None:
79 resumeProducing()83 resumeProducing()
84
85
86class DoNothingSession:
87 """A Conch user session that allows nothing."""
88
89 implements(ISession)
90
91 def __init__(self, avatar):
92 self.avatar = avatar
93
94 def closed(self):
95 """See ISession."""
96
97 def eofReceived(self):
98 """See ISession."""
99
100 def errorWithMessage(self, protocol, msg):
101 protocol.session.writeExtended(
102 connection.EXTENDED_DATA_STDERR, msg)
103 protocol.loseConnection()
104
105 def execCommand(self, protocol, command):
106 """See ISession."""
107 self.errorWithMessage(
108 protocol, "Not allowed to execute commands on this server.\r\n")
109
110 def getPty(self, term, windowSize, modes):
111 """See ISession."""
112 # Do nothing, as we don't provide shell access. openShell will get
113 # called and handle this error message and disconnect.
114
115 def openShell(self, protocol):
116 """See ISession."""
117 self.errorWithMessage(protocol, "No shells on this server.\r\n")
118
119 def windowChanged(self, newWindowSize):
120 """See ISession."""
121 raise NotImplementedError(self.windowChanged)
80122
=== added file 'lib/lp/services/sshserver/tests/test_session.py'
--- lib/lp/services/sshserver/tests/test_session.py 1970-01-01 00:00:00 +0000
+++ lib/lp/services/sshserver/tests/test_session.py 2010-06-25 17:21:32 +0000
@@ -0,0 +1,105 @@
1# Copyright 2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4"""Tests for generic SSH session support."""
5
6__metaclass__ = type
7
8import unittest
9
10from twisted.conch.interfaces import ISession
11from twisted.conch.ssh import connection
12
13from lp.services.sshserver.session import DoNothingSession
14from lp.testing import TestCase
15
16
17class MockSSHSession:
18 """Just enough of SSHSession to allow checking of reporting to stderr."""
19
20 def __init__(self, log):
21 self.log = log
22
23 def writeExtended(self, channel, data):
24 self.log.append(('writeExtended', channel, data))
25
26
27class MockProcessTransport:
28 """Mock transport used to fake speaking with child processes that are
29 mocked out in tests.
30 """
31
32 def __init__(self, executable):
33 self._executable = executable
34 self.log = []
35 self.session = MockSSHSession(self.log)
36
37 def closeStdin(self):
38 self.log.append(('closeStdin',))
39
40 def loseConnection(self):
41 self.log.append(('loseConnection',))
42
43 def signalProcess(self, signal):
44 self.log.append(('signalProcess', signal))
45
46 def write(self, data):
47 self.log.append(('write', data))
48
49
50class TestDoNothing(TestCase):
51 """Tests for ExecOnlySession."""
52
53 def setUp(self):
54 super(TestDoNothing, self).setUp()
55 self.session = DoNothingSession(None)
56
57 def test_getPtyIsANoOp(self):
58 # getPty is called on the way to establishing a shell. Since we don't
59 # give out shells, it should be a no-op. Raising an exception would
60 # log an OOPS, so we won't do that.
61 self.assertEqual(None, self.session.getPty(None, None, None))
62
63 def test_openShellNotImplemented(self):
64 # openShell closes the connection.
65 protocol = MockProcessTransport('bash')
66 self.session.openShell(protocol)
67 self.assertEqual(
68 [('writeExtended', connection.EXTENDED_DATA_STDERR,
69 'No shells on this server.\r\n'),
70 ('loseConnection',)],
71 protocol.log)
72
73 def test_windowChangedNotImplemented(self):
74 # windowChanged raises a NotImplementedError. It doesn't matter what
75 # we pass it.
76 self.assertRaises(NotImplementedError,
77 self.session.windowChanged, None)
78
79 def test_providesISession(self):
80 # ExecOnlySession must provide ISession.
81 self.failUnless(ISession.providedBy(self.session),
82 "ExecOnlySession doesn't implement ISession")
83
84 def test_closedDoesNothing(self):
85 # closed is a no-op.
86 self.assertEqual(None, self.session.closed())
87
88 def test_execCommandNotImplemented(self):
89 # ExecOnlySession.execCommand spawns the appropriate process.
90 protocol = MockProcessTransport('bash')
91 command = 'cat /etc/hostname'
92 self.session.execCommand(protocol, command)
93 self.assertEqual(
94 [('writeExtended', connection.EXTENDED_DATA_STDERR,
95 'Not allowed to execute commands on this server.\r\n'),
96 ('loseConnection',)],
97 protocol.log)
98
99 def test_eofReceivedDoesNothingWhenNoCommand(self):
100 # When no process has been created, 'eofReceived' is a no-op.
101 self.assertEqual(None, self.session.eofReceived())
102
103
104def test_suite():
105 return unittest.TestLoader().loadTestsFromName(__name__)