From bb5f6362c5e01ebfabdffb9b39c07c807cf3be8f Mon Sep 17 00:00:00 2001 From: Carlos Nihelton Date: Fri, 3 Feb 2023 16:27:38 -0300 Subject: [PATCH 1/7] Adds vocabulary type for Active Directory support ADConnectionInfo to be the model data submited by clients ADValidationResult models the enum returned from a POST request. (input must be validated in the server) --- subiquity/common/types.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/subiquity/common/types.py b/subiquity/common/types.py index 6663cf35..29b86fa5 100644 --- a/subiquity/common/types.py +++ b/subiquity/common/types.py @@ -745,3 +745,21 @@ class MirrorCheckResponse: url: str status: MirrorCheckStatus output: str + + +@attr.s(auto_attribs=True) +class ADConnectionInfo: + admin_name: str = "" + domain_name: str = "" + password: str = attr.ib(repr=False, default="") + + +class ADValidationResult(enum.Enum): + OK = enum.auto() + ADMIN_NAME_BAD_FIRST_CHAR = enum.auto() + ADMIN_NAME_BAD_CHARS = enum.auto() + DCNAME_BAD_CHARS = enum.auto() + DCNAME_BAD_HYPHEN = enum.auto() + DCNAME_BAD_DOTS = enum.auto() + DCNAME_BAD_LENGTH = enum.auto() + PASSWORD_EMPTY = enum.auto() From ceda86031f5ab8e7344c0dbba31c36544a42cc16 Mon Sep 17 00:00:00 2001 From: Carlos Nihelton Date: Mon, 6 Feb 2023 16:42:54 -0300 Subject: [PATCH 2/7] Defines the first API endpoint /active_directory Clients GET the current model or POST a new one. Not implemented yet. --- subiquity/common/apidef.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/subiquity/common/apidef.py b/subiquity/common/apidef.py index ac43348b..356706a3 100644 --- a/subiquity/common/apidef.py +++ b/subiquity/common/apidef.py @@ -25,6 +25,8 @@ from subiquitycore.models.network import ( from subiquity.common.api.defs import api, Payload, simple_endpoint from subiquity.common.types import ( + ADValidationResult, + ADConnectionInfo, AddPartitionV2, AnyStep, ApplicationState, @@ -403,6 +405,11 @@ class API: class integrity: def GET() -> CasperMd5Results: ... + class active_directory: + def GET() -> Optional[ADConnectionInfo]: ... + def POST(data: Payload[ADConnectionInfo]) \ + -> List[ADValidationResult]: ... + class LinkAction(enum.Enum): NEW = enum.auto() From 70819367de94afec82a0cce88cd36c61d0f58fbf Mon Sep 17 00:00:00 2001 From: Carlos Nihelton Date: Mon, 6 Feb 2023 16:49:52 -0300 Subject: [PATCH 3/7] Implements AD Model and Server Controller Plus the first lines of test. **Notice that the AD Controller is not required.** POST'ing still incomplete: just a placeholder method to enable cycling between testing and writing code. Now testing the API is possible. --- subiquity/models/ad.py | 33 ++++++++++++++++++++ subiquity/models/subiquity.py | 2 ++ subiquity/server/controllers/__init__.py | 2 ++ subiquity/server/controllers/ad.py | 39 ++++++++++++++++++++++++ subiquity/server/server.py | 1 + subiquity/tests/api/test_api.py | 18 +++++++++++ 6 files changed, 95 insertions(+) create mode 100644 subiquity/models/ad.py create mode 100644 subiquity/server/controllers/ad.py diff --git a/subiquity/models/ad.py b/subiquity/models/ad.py new file mode 100644 index 00000000..da79701d --- /dev/null +++ b/subiquity/models/ad.py @@ -0,0 +1,33 @@ +# Copyright 2023 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 . + +import logging +from typing import Optional + +from subiquity.common.types import ADConnectionInfo + +log = logging.getLogger('subiquity.models.ad') + + +class ADModel: + """ Models the Active Directory feature """ + def __init__(self) -> None: + self.do_join = False + self.conn_info: Optional[ADConnectionInfo] = None + + async def target_packages(self): + # NOTE Those packages must be present in the target system to allow + # joining to a domain. + return ["adcli", "realmd", "sssd"] if self.do_join else [] diff --git a/subiquity/models/subiquity.py b/subiquity/models/subiquity.py index 78fe8913..cd3c29a3 100644 --- a/subiquity/models/subiquity.py +++ b/subiquity/models/subiquity.py @@ -46,6 +46,7 @@ from subiquitycore.lsb_release import lsb_release from subiquity.common.resources import get_users_and_groups from subiquity.server.types import InstallerChannels +from .ad import ADModel from .codecs import CodecsModel from .drivers import DriversModel from .filesystem import FilesystemModel @@ -175,6 +176,7 @@ class SubiquityModel: self.target = root self.chroot_prefix = [] + self.ad = ADModel() self.codecs = CodecsModel() self.debconf_selections = DebconfSelectionsModel() self.drivers = DriversModel() diff --git a/subiquity/server/controllers/__init__.py b/subiquity/server/controllers/__init__.py index bbe8191b..7353ad3d 100644 --- a/subiquity/server/controllers/__init__.py +++ b/subiquity/server/controllers/__init__.py @@ -13,6 +13,7 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . +from .ad import ADController from .cmdlist import EarlyController, LateController, ErrorController from .codecs import CodecsController from .debconf import DebconfController @@ -41,6 +42,7 @@ from .userdata import UserdataController from .zdev import ZdevController __all__ = [ + 'ADController', 'CodecsController', 'DebconfController', 'DriversController', diff --git a/subiquity/server/controllers/ad.py b/subiquity/server/controllers/ad.py new file mode 100644 index 00000000..df607600 --- /dev/null +++ b/subiquity/server/controllers/ad.py @@ -0,0 +1,39 @@ +# Copyright 2023 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 . + +import logging +from typing import List, Optional + +from subiquity.common.apidef import API +from subiquity.common.types import ADConnectionInfo, ADValidationResult +from subiquity.server.controller import SubiquityController + +log = logging.getLogger('subiquity.server.controllers.ad') + + +class ADController(SubiquityController): + """ Implements the server part of the Active Directory feature. """ + model_name = "ad" + endpoint = API.active_directory + # No auto install key and schema for now due password handling uncertainty. + + async def GET(self) -> Optional[ADConnectionInfo]: + """Returns the currently configured AD settings""" + return self.model.conn_info + + async def POST(self, data: ADConnectionInfo) -> List[ADValidationResult]: + self.model.conn_info = data + await self.configured() + return [ADValidationResult.OK] diff --git a/subiquity/server/server.py b/subiquity/server/server.py index a2337417..5cd819a0 100644 --- a/subiquity/server/server.py +++ b/subiquity/server/server.py @@ -256,6 +256,7 @@ class SubiquityServer(Application): "Identity", "SSH", "SnapList", + "AD", "Codecs", "Drivers", "TimeZone", diff --git a/subiquity/tests/api/test_api.py b/subiquity/tests/api/test_api.py index 2b58578a..6e407ec3 100644 --- a/subiquity/tests/api/test_api.py +++ b/subiquity/tests/api/test_api.py @@ -1632,3 +1632,21 @@ class TestWSLSetupOptions(TestAPI): resp = await inst.get(endpoint) self.assertFalse(resp['install_language_support_packages']) + + +class TestActiveDirectory(TestAPI): + async def test_ad(self): + async with start_server('examples/simple.json') as instance: + endpoint = '/active_directory' + ad_dict = await instance.get(endpoint) + # Starts empty + self.assertIsNone(ad_dict) + + # POST succeeds + ad_dict = { + 'domain_name': 'ubuntu.com', + 'admin_name': 'u', + 'password': 'u' + } + result = await instance.post(endpoint, ad_dict) + self.assertIn('OK', result) From 0ea977a6cc33268350e1720cf5ee7e69d40504da Mon Sep 17 00:00:00 2001 From: Carlos Nihelton Date: Mon, 6 Feb 2023 22:37:16 -0300 Subject: [PATCH 4/7] Test enhancements: - timeout - assert equals because the success case wil return a very specific list of a single value: 'OK' --- subiquity/tests/api/test_api.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/subiquity/tests/api/test_api.py b/subiquity/tests/api/test_api.py index 6e407ec3..b9190f2d 100644 --- a/subiquity/tests/api/test_api.py +++ b/subiquity/tests/api/test_api.py @@ -1635,6 +1635,7 @@ class TestWSLSetupOptions(TestAPI): class TestActiveDirectory(TestAPI): + @timeout() async def test_ad(self): async with start_server('examples/simple.json') as instance: endpoint = '/active_directory' @@ -1649,4 +1650,4 @@ class TestActiveDirectory(TestAPI): 'password': 'u' } result = await instance.post(endpoint, ad_dict) - self.assertIn('OK', result) + self.assertEqual(['OK'], result) From fe25d07ecab1b236cecba2815954cdf56ea70e93 Mon Sep 17 00:00:00 2001 From: Carlos Nihelton Date: Mon, 6 Feb 2023 22:52:49 -0300 Subject: [PATCH 5/7] Explains the return value of active_directory.POST Returning a list of errors seems uncommon. --- subiquity/common/apidef.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/subiquity/common/apidef.py b/subiquity/common/apidef.py index 356706a3..3e98b598 100644 --- a/subiquity/common/apidef.py +++ b/subiquity/common/apidef.py @@ -407,6 +407,11 @@ class API: class active_directory: def GET() -> Optional[ADConnectionInfo]: ... + # POST must validate the payload before configuring the controller, + # which may contain several errors as described in [ADValidationResult] + # simultaneously - such as invalid chars on the admin name and DC name + # starting with a hyphen or a dot. Thus this must returns a List + # of errors [ADValidationResult.OK] on success. def POST(data: Payload[ADConnectionInfo]) \ -> List[ADValidationResult]: ... From f77971238f179cbcb0db8b16279157ef6801db28 Mon Sep 17 00:00:00 2001 From: Carlos Nihelton Date: Tue, 7 Feb 2023 14:17:49 -0300 Subject: [PATCH 6/7] Clearer logic on ADModel.target_packages method. --- subiquity/models/ad.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/subiquity/models/ad.py b/subiquity/models/ad.py index da79701d..eca617c3 100644 --- a/subiquity/models/ad.py +++ b/subiquity/models/ad.py @@ -30,4 +30,7 @@ class ADModel: async def target_packages(self): # NOTE Those packages must be present in the target system to allow # joining to a domain. - return ["adcli", "realmd", "sssd"] if self.do_join else [] + if self.do_join is True: + return ["adcli", "realmd", "sssd"] + + return [] From 7962bdcc9f0a86c4dc7f95a99e00cc0d1b875b5b Mon Sep 17 00:00:00 2001 From: Carlos Nihelton Date: Wed, 8 Feb 2023 09:05:00 -0300 Subject: [PATCH 7/7] Shorter if clause --- subiquity/models/ad.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/subiquity/models/ad.py b/subiquity/models/ad.py index eca617c3..559ba4c3 100644 --- a/subiquity/models/ad.py +++ b/subiquity/models/ad.py @@ -30,7 +30,7 @@ class ADModel: async def target_packages(self): # NOTE Those packages must be present in the target system to allow # joining to a domain. - if self.do_join is True: + if self.do_join: return ["adcli", "realmd", "sssd"] return []