Merge pull request #828 from mwhudson/simple-view-types

convert proxy, mirror, identity, ssh views to only refer to simple types

subiquity/common/types.py

@@ -0,0 +1,37 @@
+# Copyright 2020 Canonical, Ltd.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as
+# published by the Free Software Foundation, either version 3 of the
+# License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This module defines types that will be used in the API when subiquity gets
+# split into client and server processes.  View code should only use these
+# types!
+
+from typing import List
+
+import attr
+
+
+@attr.s(auto_attribs=True)
+class IdentityData:
+    realname: str = ''
+    username: str = ''
+    crypted_password: str = attr.ib(default='', repr=False)
+    hostname: str = ''
+
+
+@attr.s(auto_attribs=True)
+class SSHData:
+    install_server: bool
+    allow_pw: bool
+    authorized_keys: List[str] = attr.Factory(list)

subiquity/controllers/identity.py

@@ -19,6 +19,7 @@ import attr
 
 from subiquitycore.context import with_context
 
+from subiquity.common.types import IdentityData
 from subiquity.controller import SubiquityTuiController
 from subiquity.ui.views import IdentityView
 
@@ -42,7 +43,13 @@ class IdentityController(SubiquityTuiController):
 
     def load_autoinstall_data(self, data):
         if data is not None:
-            self.model.add_user(data)
+            identity_data = IdentityData(
+                realname=data.get('realname', ''),
+                username=data['username'],
+                hostname=data['hostname'],
+                crypted_password=data['password'],
+                )
+            self.model.add_user(identity_data)
 
     @with_context()
     async def apply_autoinstall_config(self, context=None):
@@ -51,29 +58,32 @@ class IdentityController(SubiquityTuiController):
                 raise Exception("no identity data provided")
 
     def make_ui(self):
-        return IdentityView(self.model, self)
+        data = IdentityData()
+        if self.model.user is not None:
+            data.username = self.model.user.username
+            data.realname = self.model.user.realname
+        if self.model.hostname:
+            data.hostname = self.model.hostname
+        return IdentityView(self, data)
 
     def run_answers(self):
         if all(elem in self.answers for elem in
                ['realname', 'username', 'password', 'hostname']):
-            d = {
+            identity = IdentityData(
-                'realname': self.answers['realname'],
+                realname=self.answers['realname'],
-                'username': self.answers['username'],
+                username=self.answers['username'],
-                'hostname': self.answers['hostname'],
+                hostname=self.answers['hostname'],
-                'password': self.answers['password'],
+                crypted_password=self.answers['password'])
-                }
+            self.done(identity)
-            self.done(d)
 
     def cancel(self):
         self.app.prev_screen()
 
-    def done(self, user_spec):
+    def done(self, identity_data):
-        safe_spec = user_spec.copy()
-        safe_spec['password'] = '<REDACTED>'
         log.debug(
             "IdentityController.done next_screen user_spec=%s",
-            safe_spec)
+            identity_data)
-        self.model.add_user(user_spec)
+        self.model.add_user(identity_data)
         self.configured()
         self.app.next_screen()
 

subiquity/controllers/mirror.py

@@ -124,7 +124,7 @@ class MirrorController(SubiquityTuiController):
 
     def make_ui(self):
         self.check_state = CheckState.DONE
-        return MirrorView(self.model, self)
+        return MirrorView(self, self.model.get_mirror())
 
     def run_answers(self):
         if 'mirror' in self.answers:

subiquity/controllers/proxy.py

@@ -49,7 +49,7 @@ class ProxyController(SubiquityTuiController):
         pass
 
     def make_ui(self):
-        return ProxyView(self.model, self)
+        return ProxyView(self, self.model.proxy)
 
     def run_answers(self):
         if 'proxy' in self.answers:

subiquity/controllers/ssh.py

@@ -20,6 +20,7 @@ from subiquitycore.async_helpers import schedule_task
 from subiquitycore.context import with_context
 from subiquitycore import utils
 
+from subiquity.common.types import SSHData
 from subiquity.controller import SubiquityTuiController
 from subiquity.ui.views.ssh import SSHView
 
@@ -66,25 +67,25 @@ class SSHController(SubiquityTuiController):
             'allow-pw', not self.model.authorized_keys)
 
     def make_ui(self):
-        return SSHView(self.model, self)
+        ssh_data = SSHData(
+            install_server=self.model.install_server,
+            allow_pw=self.model.pwauth)
+        return SSHView(self, ssh_data)
 
     def run_answers(self):
         if 'ssh-import-id' in self.answers:
             import_id = self.answers['ssh-import-id']
-            d = {
+            ssh = SSHData(
-                "ssh_import_id": import_id.split(":", 1)[0],
+                install_server=True,
-                "import_username": import_id.split(":", 1)[1],
+                authorized_keys=[],
-                "install_server": True,
+                allow_pw=True)
-                "pwauth": True,
+            self.fetch_ssh_keys(ssh_import_id=import_id, ssh_data=ssh)
-            }
-            self.fetch_ssh_keys(d)
         else:
-            d = {
+            ssh = SSHData(
-                "install_server": self.answers.get("install_server", False),
+                install_server=self.answers.get("install_server", False),
-                "authorized_keys": self.answers.get("authorized_keys", []),
+                authorized_keys=self.answers.get("authorized_keys", []),
-                "pwauth": self.answers.get("pwauth", True),
+                allow_pw=self.answers.get("pwauth", True))
-            }
+            self.done(ssh)
-            self.done(d)
 
     def cancel(self):
         self.app.prev_screen()
@@ -103,10 +104,8 @@ class SSHController(SubiquityTuiController):
         return cp
 
     @with_context(
-        name="ssh_import_id",
+        name="ssh_import_id", description="{ssh_import_id}")
-        description="{user_spec[ssh_import_id]}:{user_spec[import_username]}")
+    async def _fetch_ssh_keys(self, *, context, ssh_import_id, ssh_data):
-    async def _fetch_ssh_keys(self, *, context, user_spec):
-        ssh_import_id = "{ssh_import_id}:{import_username}".format(**user_spec)
         with self.context.child("ssh_import_id", ssh_import_id):
             try:
                 cp = await self.run_cmd_checked(
@@ -131,22 +130,21 @@ class SSHController(SubiquityTuiController):
                 "").strip().splitlines()
 
             if 'ssh-import-id' in self.app.answers.get("Identity", {}):
-                user_spec['authorized_keys'] = key_material.splitlines()
+                ssh_data.authorized_keys = key_material.splitlines()
-                self.done(user_spec)
+                self.done(ssh_data)
             else:
                 self.ui.body.confirm_ssh_keys(
-                    user_spec, ssh_import_id, key_material, fingerprints)
+                    ssh_data, ssh_import_id, key_material, fingerprints)
 
-    def fetch_ssh_keys(self, user_spec):
+    def fetch_ssh_keys(self, ssh_import_id, ssh_data):
         self._fetch_task = schedule_task(
-            self._fetch_ssh_keys(user_spec=user_spec))
+            self._fetch_ssh_keys(
+                ssh_import_id=ssh_import_id, ssh_data=ssh_data))
 
-    def done(self, result):
+    def done(self, data: SSHData):
-        log.debug("SSHController.done next_screen result=%s", result)
+        self.model.install_server = data.install_server
-        self.model.install_server = result['install_server']
+        self.model.authorized_keys = data.authorized_keys
-        self.model.authorized_keys = result.get('authorized_keys', [])
+        self.model.pwauth = data.allow_pw
-        self.model.pwauth = result.get('pwauth', True)
-        self.model.ssh_import_id = result.get('ssh_import_id', None)
         self.configured()
         self.app.next_screen()
 

subiquity/models/identity.py

@@ -17,8 +17,6 @@ import logging
 
 import attr
 
-from subiquitycore.utils import crypt_password
-
 
 log = logging.getLogger('subiquity.models.identity')
 
@@ -38,12 +36,15 @@ class IdentityModel(object):
         self._user = None
         self._hostname = None
 
-    def add_user(self, result):
+    def add_user(self, identity_data):
-        result = result.copy()
+        self._hostname = identity_data.hostname
-        self._hostname = result.pop('hostname')
+        d = {}
-        if not result.get('realname'):
+        d['realname'] = identity_data.realname
-            result['realname'] = result['username']
+        d['username'] = identity_data.username
-        self._user = User(**result)
+        d['password'] = identity_data.crypted_password
+        if not d['realname']:
+            d['realname'] = identity_data.username
+        self._user = User(**d)
 
     @property
     def hostname(self):
@@ -53,8 +54,5 @@ class IdentityModel(object):
     def user(self):
         return self._user
 
-    def encrypt_password(self, passinput):
-        return crypt_password(passinput)
-
     def __repr__(self):
         return "<LocalUser: {} {}>".format(self.user, self.hostname)

subiquity/ui/views/identity.py

@@ -31,8 +31,11 @@ from subiquitycore.ui.form import (
     WantsToKnowFormField,
     )
 from subiquitycore.ui.utils import screen
+from subiquitycore.utils import crypt_password
 from subiquitycore.view import BaseView
 
+from subiquity.common.types import IdentityData
+
 
 log = logging.getLogger("subiquity.views.identity")
 
@@ -153,10 +156,8 @@ class IdentityView(BaseView):
                 "the system. You can configure SSH access on the next screen "
                 "but a password is still needed for sudo.")
 
-    def __init__(self, model, controller):
+    def __init__(self, controller, identity_data):
-        self.model = model
         self.controller = controller
-        self.signal = controller.signal
 
         reserved_usernames_path = (
             os.path.join(os.environ.get("SNAP", "."), "reserved-usernames"))
@@ -171,14 +172,11 @@ class IdentityView(BaseView):
         else:
             reserved_usernames.add('root')
 
-        if model.user:
+        initial = {
-            initial = {
+            'realname': identity_data.realname,
-                'realname': model.user.realname,
+            'username': identity_data.username,
-                'username': model.user.username,
+            'hostname': identity_data.hostname,
-                'hostname': model.hostname,
             }
-        else:
-            initial = {}
 
         self.form = IdentityForm(reserved_usernames, initial)
 
@@ -193,10 +191,9 @@ class IdentityView(BaseView):
                 focus_buttons=False))
 
     def done(self, result):
-        result = {
+        self.controller.done(IdentityData(
-            "hostname": self.form.hostname.value,
+            hostname=self.form.hostname.value,
-            "realname": self.form.realname.value,
+            realname=self.form.realname.value,
-            "username": self.form.username.value,
+            username=self.form.username.value,
-            "password": self.model.encrypt_password(self.form.password.value),
+            crypted_password=crypt_password(self.form.password.value),
-        }
+            ))
-        self.controller.done(result)

subiquity/ui/views/mirror.py

@@ -46,11 +46,10 @@ class MirrorView(BaseView):
     excerpt = _("If you use an alternative mirror for Ubuntu, enter its "
                 "details here.")
 
-    def __init__(self, model, controller):
+    def __init__(self, controller, mirror):
-        self.model = model
         self.controller = controller
 
-        self.form = MirrorForm(initial={'url': self.model.get_mirror()})
+        self.form = MirrorForm(initial={'url': mirror})
 
         connect_signal(self.form, 'submit', self.done)
         connect_signal(self.form, 'cancel', self.cancel)

subiquity/ui/views/proxy.py

@@ -49,11 +49,10 @@ class ProxyView(BaseView):
     excerpt = _("If this system requires a proxy to connect to the internet, "
                 "enter its details here.")
 
-    def __init__(self, model, controller):
+    def __init__(self, controller, proxy):
-        self.model = model
         self.controller = controller
 
-        self.form = ProxyForm(initial={'url': self.model.proxy})
+        self.form = ProxyForm(initial={'url': proxy})
 
         connect_signal(self.form, 'submit', self.done)
         connect_signal(self.form, 'cancel', self.cancel)

subiquity/ui/views/ssh.py

@@ -51,6 +51,7 @@ from subiquitycore.ui.utils import (
     SomethingFailed,
     )
 
+from subiquity.common.types import SSHData
 from subiquity.ui.views.identity import (
     UsernameField,
     )
@@ -176,11 +177,9 @@ class FetchingSSHKeys(WidgetWrap):
 
 
 class ConfirmSSHKeys(Stretchy):
-    def __init__(self, parent, result, ssh_import_id, key_material,
+    def __init__(self, parent, ssh_data, key_material, fingerprints):
-                 fingerprints):
         self.parent = parent
-        self.result = result
+        self.ssh_data = ssh_data
-        self.ssh_import_id = ssh_import_id
         self.key_material = key_material
 
         ok = ok_btn(label=_("Yes"), on_press=self.ok)
@@ -212,9 +211,8 @@ class ConfirmSSHKeys(Stretchy):
         self.parent.remove_overlay()
 
     def ok(self, sender):
-        self.result['authorized_keys'] = self.key_material.splitlines()
+        self.ssh_data.authorized_keys = self.key_material.splitlines()
-        self.result['ssh_import_id'] = self.ssh_import_id
+        self.parent.controller.done(self.ssh_data)
-        self.parent.controller.done(self.result)
 
 
 class SSHView(BaseView):
@@ -223,18 +221,13 @@ class SSHView(BaseView):
     excerpt = _("You can choose to install the OpenSSH server package to "
                 "enable secure remote access to your server.")
 
-    def __init__(self, model, controller):
+    def __init__(self, controller, ssh_data):
-        self.model = model
         self.controller = controller
 
         initial = {
-            "install_server": self.model.install_server,
+            "install_server": ssh_data.install_server,
-            "pwauth": self.model.pwauth,
+            "pwauth": ssh_data.allow_pw,
             }
-        if self.model.ssh_import_id:
-            prefix, username = self.model.ssh_import_id.split(':', 1)
-            initial['ssh_import_id'] = prefix
-            initial['import_username'] = username
 
         self.form = SSHForm(initial=initial)
 
@@ -276,25 +269,29 @@ class SSHView(BaseView):
             iu.validate()
 
     def done(self, sender):
-        result = self.form.as_data()
+        log.debug("User input: {}".format(self.form.as_data()))
-        log.debug("User input: {}".format(result))
+        ssh_data = SSHData(
+            install_server=self.form.install_server.value,
+            allow_pw=self.form.pwauth.value)
 
         # if user specifed a value, allow user to validate fingerprint
         if self.form.ssh_import_id.value:
+            ssh_import_id = self.form.ssh_import_id.value + ":" + \
+              self.form.import_username.value
             fsk = FetchingSSHKeys(self)
             self.show_overlay(fsk, width=fsk.width, min_width=None)
-            self.controller.fetch_ssh_keys(result)
+            self.controller.fetch_ssh_keys(
+                ssh_import_id=ssh_import_id, ssh_data=ssh_data)
         else:
-            self.controller.done(result)
+            self.controller.done(ssh_data)
 
     def cancel(self, result=None):
         self.controller.cancel()
 
-    def confirm_ssh_keys(self, result, ssh_import_id, ssh_key, fingerprints):
+    def confirm_ssh_keys(self, ssh_data, ssh_import_id, ssh_key, fingerprints):
         self.remove_overlay()
         self.show_stretchy_overlay(
-            ConfirmSSHKeys(
+            ConfirmSSHKeys(self, ssh_data, ssh_key, fingerprints))
-                self, result, ssh_import_id, ssh_key, fingerprints))
 
     def fetching_ssh_keys_failed(self, msg, stderr):
         self.remove_overlay()

subiquity/ui/views/tests/test_identity.py

@@ -4,8 +4,8 @@ from unittest import mock
 from subiquitycore.signals import Signal
 from subiquitycore.testing import view_helpers
 
-from subiquity.models.identity import IdentityModel
 from subiquity.controllers.identity import IdentityController
+from subiquity.common.types import IdentityData
 from subiquity.ui.views.identity import IdentityView
 
 
@@ -21,10 +21,9 @@ valid_data = {
 class IdentityViewTests(unittest.TestCase):
 
     def make_view(self):
-        model = mock.create_autospec(spec=IdentityModel)
         controller = mock.create_autospec(spec=IdentityController)
         controller.signal = mock.create_autospec(spec=Signal)
-        return IdentityView(model, controller)
+        return IdentityView(controller, IdentityData())
 
     def test_initial_focus(self):
         view = self.make_view()
@@ -46,16 +45,17 @@ class IdentityViewTests(unittest.TestCase):
 
     def test_click_done(self):
         view = self.make_view()
-        view_helpers.enter_data(view.form, valid_data)
         CRYPTED = '<crypted>'
-        view.model.encrypt_password.side_effect = lambda p: CRYPTED
+        with mock.patch('subiquity.ui.views.identity.crypt_password') as cp:
-        expected = valid_data.copy()
+            cp.side_effect = lambda p: CRYPTED
-        expected['password'] = CRYPTED
+            view_helpers.enter_data(view.form, valid_data)
-        del expected['confirm_password']
+            done_btn = view_helpers.find_button_matching(view, "^Done$")
-
+            view_helpers.click(done_btn)
-        done_btn = view_helpers.find_button_matching(view, "^Done$")
+        expected = IdentityData(
-        view_helpers.click(done_btn)
+            realname=valid_data['realname'],
-
+            username=valid_data['username'],
+            hostname=valid_data['hostname'],
+            crypted_password=CRYPTED)
         view.controller.done.assert_called_once_with(expected)
 
     def test_can_tab_to_done_when_valid(self):