Merge ~ltrager/maas:vcenter_creds_ui into maas:master

Proposed by Lee Trager
Status: Merged
Approved by: Lee Trager
Approved revision: 95e5c8e31c89c86b7928e9a01d5611af7128d31a
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~ltrager/maas:vcenter_creds_ui
Merge into: maas:master
Diff against target: 127 lines (+52/-2)
4 files modified
src/maasserver/forms/__init__.py (+9/-0)
src/maasserver/templates/maasserver/settings_general.html (+16/-0)
src/maasserver/views/settings.py (+10/-1)
src/maasserver/views/tests/test_settings.py (+17/-1)
Reviewer Review Type Date Requested Status
Björn Tillenius Approve
Review via email: mp+365985@code.launchpad.net

Commit message

Allow settings vCenter credentials on the settings page in the UI.

To post a comment you must log in.
~ltrager/maas:vcenter_creds_ui updated
95e5c8e... by Lee Trager

s/windows/vcenter/g

Revision history for this message
Björn Tillenius (bjornt) wrote :

The code itself looks good. But i'd like to take the opportunity to clarify how this is expected to work.

Do we have the workflow written down somewhere?

I'm a bit concerned that the description says that the credentials are being sent to the deployed how. So how do we prevent a normal user to be able to access those credentials?

review: Needs Information
Revision history for this message
Andres Rodriguez (andreserl) wrote :

Those credentials are sent to the node via the bender data, the same way we
send other administrative data that no other users have access.

This is the same way you wouldn’t send landscape credentials via cloud-init
for auto config.

On Mon, Apr 15, 2019 at 3:44 AM Björn Tillenius <email address hidden> wrote:

> Review: Needs Information
>
> The code itself looks good. But i'd like to take the opportunity to
> clarify how this is expected to work.
>
> Do we have the workflow written down somewhere?
>
> I'm a bit concerned that the description says that the credentials are
> being sent to the deployed how. So how do we prevent a normal user to be
> able to access those credentials?
> --
> https://code.launchpad.net/~ltrager/maas/+git/maas/+merge/365985
> Your team MAAS Committers is subscribed to branch maas:master.
>
> Launchpad-Message-Rationale: Subscriber @maas-committers
> Launchpad-Message-For: maas-committers
> Launchpad-Notification-Type: code-review
> Launchpad-Branch: ~maas-committers/maas/+git/maas:master
> Launchpad-Project: maas
>
--
Andres Rodriguez (RoAkSoAx)
Ubuntu Server Developer
MSc. Telecom & Networking
Systems Engineer

Revision history for this message
Björn Tillenius (bjornt) wrote :

So I think this deserves a write-up to discuss what attack vector we protect against, and which we don't. Sending anything to the nodes open up a range of attach vectors, and considering these are shared global credentials, we should be clear on how they can be exposed to users that shouldn't have access to them.

Revision history for this message
Andres Rodriguez (andreserl) wrote :

That however, sits outside of the scope of this branch!

On Mon, Apr 15, 2019 at 4:51 AM Björn Tillenius <email address hidden> wrote:

> So I think this deserves a write-up to discuss what attack vector we
> protect against, and which we don't. Sending anything to the nodes open up
> a range of attach vectors, and considering these are shared global
> credentials, we should be clear on how they can be exposed to users that
> shouldn't have access to them.
> --
> https://code.launchpad.net/~ltrager/maas/+git/maas/+merge/365985
> Your team MAAS Committers is subscribed to branch maas:master.
>
> Launchpad-Message-Rationale: Subscriber @maas-committers
> Launchpad-Message-For: maas-committers
> Launchpad-Notification-Type: code-review
> Launchpad-Branch: ~maas-committers/maas/+git/maas:master
> Launchpad-Project: maas
>
--
Andres Rodriguez (RoAkSoAx)
Ubuntu Server Developer
MSc. Telecom & Networking
Systems Engineer

Revision history for this message
Björn Tillenius (bjornt) wrote :

We discussed this on the standup. As mentioned, my comments weren't on this branch in particular, but rather on the feature as whole.

So I'm approving this branch, and then Lee can update the spec and make it so that only admins and operators can deploy machines using these credentials.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/maasserver/forms/__init__.py b/src/maasserver/forms/__init__.py
2index d1e653c..4d90cea 100644
3--- a/src/maasserver/forms/__init__.py
4+++ b/src/maasserver/forms/__init__.py
5@@ -62,6 +62,7 @@ __all__ = [
6 "UpdateRaidForm",
7 "UpdateVirtualBlockDeviceForm",
8 "UpdateVolumeGroupForm",
9+ "VCenterForm",
10 "WindowsForm",
11 "ZoneForm",
12 ]
13@@ -1736,6 +1737,14 @@ class WindowsForm(ConfigForm):
14 windows_kms_host = get_config_field('windows_kms_host')
15
16
17+class VCenterForm(ConfigForm):
18+ """Settings page, VMware vCenter section."""
19+ vcenter_server = get_config_field('vcenter_server')
20+ vcenter_username = get_config_field('vcenter_username')
21+ vcenter_password = get_config_field('vcenter_password')
22+ vcenter_datacenter = get_config_field('vcenter_datacenter')
23+
24+
25 class GlobalKernelOptsForm(ConfigForm):
26 """Settings page, Global Kernel Parameters section."""
27 kernel_opts = get_config_field('kernel_opts')
28diff --git a/src/maasserver/templates/maasserver/settings_general.html b/src/maasserver/templates/maasserver/settings_general.html
29index 636f153..5f5e97d 100644
30--- a/src/maasserver/templates/maasserver/settings_general.html
31+++ b/src/maasserver/templates/maasserver/settings_general.html
32@@ -130,6 +130,22 @@
33
34 <div class="p-strip is-bordered">
35 <div class="row">
36+ <div id="vcenter" class="col-8">
37+ <h2 class="p-heading--four">VMware vCenter</h2>
38+ <form action="{% url 'settings_general' %}" method="post">
39+ {% csrf_token %}
40+ <ul class="p-list">
41+ {% for field in vcenter_form %} {% include "maasserver/form_field.html" %} {% endfor %}
42+ </ul>
43+ <input type="hidden" name="vcenter_submit" value="1" />
44+ <button type="submit" class="p-button--positive u-float--right">Save</button>
45+ </form>
46+ </div>
47+ </div>
48+ </div>
49+
50+ <div class="p-strip is-bordered">
51+ <div class="row">
52 <div id="third_party_drivers" class="col-8">
53 <h2 class="p-heading--four">Third Party Drivers Configuration</h2>
54 <form action="{% url 'settings_general' %}" method="post">
55diff --git a/src/maasserver/views/settings.py b/src/maasserver/views/settings.py
56index 9d3ef97..d22f015 100644
57--- a/src/maasserver/views/settings.py
58+++ b/src/maasserver/views/settings.py
59@@ -1,4 +1,4 @@
60-# Copyright 2012-2018 Canonical Ltd. This software is licensed under the
61+# Copyright 2012-2019 Canonical Ltd. This software is licensed under the
62 # GNU Affero General Public License version 3 (see the file LICENSE).
63
64 """Settings views."""
65@@ -47,6 +47,7 @@ from maasserver.forms import (
66 SyslogForm,
67 ThirdPartyDriversForm,
68 UbuntuForm,
69+ VCenterForm,
70 WindowsForm,
71 )
72 from maasserver.models import (
73@@ -283,6 +284,13 @@ def general(request):
74 if response is not None:
75 return response
76
77+ # Process the VMware vCenter form.
78+ vcenter_form, response = process_form(
79+ request, VCenterForm, reverse('settings_general'), 'vcenter',
80+ "Configuration updated.")
81+ if response is not None:
82+ return response
83+
84 # Process the Global Kernel Opts form.
85 kernelopts_form, response = process_form(
86 request, GlobalKernelOptsForm, reverse('settings_general'),
87@@ -300,6 +308,7 @@ def general(request):
88 'deploy_form': deploy_form,
89 'ubuntu_form': ubuntu_form,
90 'windows_form': windows_form,
91+ 'vcenter_form': vcenter_form,
92 'kernelopts_form': kernelopts_form,
93 'show_license_keys': show_license_keys(),
94 })
95diff --git a/src/maasserver/views/tests/test_settings.py b/src/maasserver/views/tests/test_settings.py
96index d8d0ae0..5033c6e 100644
97--- a/src/maasserver/views/tests/test_settings.py
98+++ b/src/maasserver/views/tests/test_settings.py
99@@ -1,4 +1,4 @@
100-# Copyright 2012-2018 Canonical Ltd. This software is licensed under the
101+# Copyright 2012-2019 Canonical Ltd. This software is licensed under the
102 # GNU Affero General Public License version 3 (see the file LICENSE).
103
104 """Test maasserver settings views."""
105@@ -243,6 +243,22 @@ class SettingsTest(MAASServerTestCase):
106 Config.objects.get_config('commissioning_distro_series'),
107 ))
108
109+ def test_settings_vcenter_POST(self):
110+ self.client.login(user=factory.make_admin())
111+ vcenter = {
112+ 'vcenter_server': factory.make_name('vcenter_server'),
113+ 'vcenter_username': factory.make_name('vcenter_username'),
114+ 'vcenter_password': factory.make_name('vcenter_password'),
115+ 'vcenter_datacenter': factory.make_name('vcenter_datacenter'),
116+ }
117+ response = self.client.post(
118+ reverse('settings_general'),
119+ get_prefixed_form_data(prefix='vcenter', data=vcenter),
120+ )
121+ self.assertEqual(http.client.FOUND, response.status_code)
122+ self.assertDictEqual(
123+ vcenter, Config.objects.get_configs(vcenter.keys()))
124+
125 def test_settings_hides_license_keys_if_no_OS_supporting_keys(self):
126 self.client.login(user=factory.make_admin())
127 response = self.client.get(reverse('settings_general'))

Subscribers

People subscribed via source and target branches