Merge ~mertkirpici/charm-local-users:lp/1983437 into charm-local-users:main

Proposed by Mert Kirpici
Status: Merged
Approved by: Eric Chen
Approved revision: 9f62485be1332d2359303e1f1565d9b5e38e1bc3
Merged at revision: fdc2f14b7406486ad805917e45a965547a6166b5
Proposed branch: ~mertkirpici/charm-local-users:lp/1983437
Merge into: charm-local-users:main
Diff against target: 623 lines (+338/-51)
19 files modified
Makefile (+1/-5)
config.yaml (+7/-0)
lib/local_users.py (+46/-19)
src/charm.py (+2/-1)
tests/functional/requirements.txt (+3/-0)
tests/functional/tests/bundles/base.yaml (+6/-0)
tests/functional/tests/bundles/bionic.yaml (+1/-0)
tests/functional/tests/bundles/focal.yaml (+1/-0)
tests/functional/tests/bundles/jammy.yaml (+1/-0)
tests/functional/tests/bundles/overlays/bionic.yaml.j2 (+1/-0)
tests/functional/tests/bundles/overlays/focal.yaml.j2 (+1/-0)
tests/functional/tests/bundles/overlays/jammy.yaml.j2 (+1/-0)
tests/functional/tests/bundles/overlays/local-charm-overlay.yaml.j2 (+3/-0)
tests/functional/tests/modules/__init__.py (+0/-0)
tests/functional/tests/modules/utils.py (+37/-0)
tests/functional/tests/test_local_users.py (+89/-0)
tests/functional/tests/tests.yaml (+16/-0)
tests/unit/test_local_users.py (+118/-22)
tox.ini (+4/-4)
Reviewer Review Type Date Requested Status
Eric Chen Approve
🤖 prod-jenkaas-bootstack (community) continuous-integration Approve
Gabriel Cocenza Approve
Erhan Sunar Pending
BootStack Reviewers Pending
Review via email: mp+430112@code.launchpad.net

Commit message

Close LP #1983437

Description of the change

config: add option ssh-authorized-keys

Introduce a new config option to set the path to the authorized keys file that will be used to write the ssh public key of the user for remote access. The config option also supports the usage of common variables $HOME, $USER and $UID which will be expanded for each user.

To post a comment you must log in.
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

Revision history for this message
🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote :
review: Approve (continuous-integration)
Revision history for this message
Mert Kirpici (mertkirpici) wrote :

since this MP adds the first functional tests for this repository, changed the jenkins job to include the `make functional` target and retriggered the job now. We should expect the results for that also. That will be the reason for the third comment from the ci-bot.

Revision history for this message
🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote :
review: Approve (continuous-integration)
Revision history for this message
Gabriel Cocenza (gabrielcocenza) :
review: Needs Information
Revision history for this message
Mert Kirpici (mertkirpici) wrote :

Hey Gabriel thanks for the review! I pushed an update and wrote some answers to comments inline.

Revision history for this message
🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote :
review: Approve (continuous-integration)
Revision history for this message
Gabriel Cocenza (gabrielcocenza) wrote :

Thanks for the fixing Mert. Generally the patch LGTM.

I have some comments in the code and I also would like to check with you to include lint and unit tests on the CI if possible. If I understood correctly just functional tests are running in the CI.

Revision history for this message
🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote :
review: Approve (continuous-integration)
Revision history for this message
Mert Kirpici (mertkirpici) wrote :

Hi there Gabriel, pushed an update that addresses your comments. Now we are seeing the coverage report line by line :)

Also I verified that the CI is running lint+unit+func, at the console output the lint and unittest output are only can be seen after clicking small link to show the "full output" since the functest output is huge

Revision history for this message
Gabriel Cocenza (gabrielcocenza) wrote :

Thanks Mert. LGTM

Lefty two nit comments in the unit tests

review: Approve
Revision history for this message
Eric Chen (eric-chen) wrote :

LGTM too.
After change Gabriel's last comment if it make sense to you then we can merge it. Thanks!

review: Needs Fixing
Revision history for this message
🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote :
review: Approve (continuous-integration)
Revision history for this message
Mert Kirpici (mertkirpici) wrote :

Hi Eric, I pushed a final update that:
- addresses Gabriel's comments to add assertions to chmod() calls
- squashes the commits so that it is a clean merge

The CI came clean, I think we are ready to merge

Revision history for this message
Eric Chen (eric-chen) :
review: Approve
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

Change successfully merged at revision fdc2f14b7406486ad805917e45a965547a6166b5

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/Makefile b/Makefile
index f335696..57a3fad 100644
--- a/Makefile
+++ b/Makefile
@@ -82,11 +82,7 @@ unittests:
82 @echo "Running unit tests"82 @echo "Running unit tests"
83 @tox -e unit83 @tox -e unit
8484
85snap:85functional: build
86 @echo "Downloading snap"
87 snap download juju-lint --basename juju-lint --target-directory tests/functional/tests/resources
88
89functional: build snap
90 @echo "Executing functional tests in ${CHARM_BUILD_DIR}"86 @echo "Executing functional tests in ${CHARM_BUILD_DIR}"
91 @CHARM_LOCATION=${PROJECTPATH} tox -e func87 @CHARM_LOCATION=${PROJECTPATH} tox -e func
9288
diff --git a/config.yaml b/config.yaml
index 2a7920c..3f5d8dd 100644
--- a/config.yaml
+++ b/config.yaml
@@ -30,6 +30,13 @@ options:
30 description:30 description:
31 When set to False the charm will enter 'blocked' state when user exists in 'users' config and in the system, but not in the charm managed group.31 When set to False the charm will enter 'blocked' state when user exists in 'users' config and in the system, but not in the charm managed group.
32 Setting to True disables that protection and allows for pre-existing users to be added to the charm managed group.32 Setting to True disables that protection and allows for pre-existing users to be added to the charm managed group.
33 ssh-authorized-keys:
34 default: "$HOME/.ssh/authorized_keys"
35 type: string
36 description: |
37 The file to write the SSH public keys to.
38 This option supports the usage of variables "$USER", "$HOME" and "$UID" in the path string.
39 They will be expanded to the username, home directory and the user id of each user.
33 sudoers:40 sudoers:
34 default: ""41 default: ""
35 type: string42 type: string
diff --git a/lib/local_users.py b/lib/local_users.py
index 28dd13d..efe5ec3 100755
--- a/lib/local_users.py
+++ b/lib/local_users.py
@@ -17,11 +17,14 @@
17import grp17import grp
18import logging18import logging
19import os19import os
20import pwd
20import re21import re
21import shutil22import shutil
22import subprocess23import subprocess
23import tempfile24import tempfile
24from collections import namedtuple25from collections import namedtuple
26from pathlib import Path
27from typing import Text
2528
26from charmhelpers.core import host29from charmhelpers.core import host
2730
@@ -29,7 +32,6 @@ log = logging.getLogger(__name__)
2932
30User = namedtuple("User", ["name", "gecos", "ssh_keys"])33User = namedtuple("User", ["name", "gecos", "ssh_keys"])
3134
32HOME_DIR_PATH = "/home"
33SUDOERS_FILE = "/etc/sudoers.d/70-local-users-charm"35SUDOERS_FILE = "/etc/sudoers.d/70-local-users-charm"
3436
3537
@@ -48,7 +50,7 @@ def add_user(username, shell="/bin/bash", home_dir=None, gecos=None):
48 subprocess.check_call(cmd)50 subprocess.check_call(cmd)
4951
5052
51def configure_user(user, group):53def configure_user(user, group, authorized_keys_path):
52 """Idempotently apply requested User configuration.54 """Idempotently apply requested User configuration.
5355
54 Create a new account if it doesn't exist. Ensure it belongs to the requested group.56 Create a new account if it doesn't exist. Ensure it belongs to the requested group.
@@ -63,7 +65,7 @@ def configure_user(user, group):
63 gecos=user.gecos,65 gecos=user.gecos,
64 )66 )
65 host.add_user_to_group(user.name, group)67 host.add_user_to_group(user.name, group)
66 set_ssh_authorized_keys(user)68 set_ssh_authorized_keys(user, authorized_keys_path)
67 update_gecos(user)69 update_gecos(user)
6870
6971
@@ -121,7 +123,27 @@ def rename_group(old_name, new_name):
121 subprocess.check_call(cmd)123 subprocess.check_call(cmd)
122124
123125
124def set_ssh_authorized_keys(user):126def _substitute_path_vars_for_user(path: Path, username: Text) -> Path:
127 """Substitute common variables in terms of a user.
128
129 Supports the variables $HOME, $USER and $UID.
130 """
131 passwd_entry = pwd.getpwnam(username)
132 return Path(
133 str(path)
134 .replace("$HOME", passwd_entry.pw_dir)
135 .replace("$USER", passwd_entry.pw_name)
136 .replace("$UID", str(passwd_entry.pw_uid))
137 )
138
139
140def _path_under_user_home(path: Path, username: Text) -> bool:
141 """Test whether the given path lies inside the user home."""
142 passwd_entry = pwd.getpwnam(username)
143 return str(path).startswith(passwd_entry.pw_dir + "/")
144
145
146def set_ssh_authorized_keys(user, authorized_keys_path):
125 """Idempotently set up the SSH public key in `authorized_keys`."""147 """Idempotently set up the SSH public key in `authorized_keys`."""
126 comment = "# charm-local-users"148 comment = "# charm-local-users"
127 authorized_keys = []149 authorized_keys = []
@@ -129,21 +151,24 @@ def set_ssh_authorized_keys(user):
129 for ssh_key in user.ssh_keys:151 for ssh_key in user.ssh_keys:
130 authorized_key = " ".join([ssh_key, comment])152 authorized_key = " ".join([ssh_key, comment])
131 authorized_keys.append(authorized_key)153 authorized_keys.append(authorized_key)
132 ssh_path = os.path.join(HOME_DIR_PATH, user.name, ".ssh")
133 authorized_keys_path = os.path.join(ssh_path, "authorized_keys")
134154
135 if not os.path.exists(ssh_path):155 authorized_keys_path = _substitute_path_vars_for_user(
136 os.makedirs(ssh_path, mode=0o700)156 Path(authorized_keys_path), user.name
137 os.chmod(ssh_path, 0o700)157 )
138 shutil.chown(ssh_path, user=user.name, group=user.name)158 ssh_path = authorized_keys_path.parent
159 ssh_path_under_user_home = _path_under_user_home(ssh_path, user.name)
160
161 ssh_path.mkdir(parents=True, exist_ok=True)
162
163 if ssh_path_under_user_home:
164 ssh_path.chmod(mode=0o700)
165 shutil.chown(ssh_path, user=user.name, group=user.name)
139166
140 # get currently configured keys167 # get currently configured keys
141 current_keys = []168 current_keys = []
142169
143 if os.path.exists(authorized_keys_path):170 if authorized_keys_path.exists():
144 with open(authorized_keys_path, "r") as keys_file:171 current_keys = authorized_keys_path.read_text().splitlines()
145 current_keys = keys_file.readlines()
146 keys_file.close()
147172
148 # keep the non-managed keys173 # keep the non-managed keys
149 regex = re.compile(r"charm-local-users")174 regex = re.compile(r"charm-local-users")
@@ -152,16 +177,18 @@ def set_ssh_authorized_keys(user):
152177
153 for authorized_key in authorized_keys:178 for authorized_key in authorized_keys:
154 new_keys.append(authorized_key + "\n")179 new_keys.append(authorized_key + "\n")
155 with open(authorized_keys_path, "w+") as keys_file:180 with authorized_keys_path.open("w+") as keys_file:
156 for key in new_keys:181 for key in new_keys:
157 keys_file.write(key)182 keys_file.write(key)
158 keys_file.close()
159183
160 # ensure correct permissions184 # ensure correct permissions
161185
162 if os.path.exists(authorized_keys_path):186 if authorized_keys_path.exists():
163 os.chmod(authorized_keys_path, 0o600)187 if ssh_path_under_user_home:
164 shutil.chown(authorized_keys_path, user=user.name, group=user.name)188 authorized_keys_path.chmod(mode=0o600)
189 shutil.chown(authorized_keys_path, user=user.name, group=user.name)
190 else:
191 authorized_keys_path.chmod(mode=0o644)
165192
166193
167def get_gecos(username):194def get_gecos(username):
diff --git a/src/charm.py b/src/charm.py
index 933a225..2b79caa 100755
--- a/src/charm.py
+++ b/src/charm.py
@@ -151,8 +151,9 @@ class CharmLocalUsersCharm(CharmBase):
151 delete_user(u, backup_path)151 delete_user(u, backup_path)
152152
153 # configure user accounts specified in the config153 # configure user accounts specified in the config
154 authorized_keys_path = self.config["ssh-authorized-keys"]
154 for user in userlist:155 for user in userlist:
155 configure_user(user, group)156 configure_user(user, group, authorized_keys_path)
156157
157 # Configure custom /etc/sudoers.d file158 # Configure custom /etc/sudoers.d file
158 sudoers = self.config["sudoers"]159 sudoers = self.config["sudoers"]
diff --git a/tests/functional/requirements.txt b/tests/functional/requirements.txt
159new file mode 100644160new file mode 100644
index 0000000..ea401e0
--- /dev/null
+++ b/tests/functional/requirements.txt
@@ -0,0 +1,3 @@
1git+https://github.com/openstack-charmers/zaza.git#egg=zaza
2cryptography
3python-openstackclient
0\ No newline at end of file4\ No newline at end of file
diff --git a/tests/functional/tests/bundles/base.yaml b/tests/functional/tests/bundles/base.yaml
1new file mode 1006445new file mode 100644
index 0000000..86e83a1
--- /dev/null
+++ b/tests/functional/tests/bundles/base.yaml
@@ -0,0 +1,6 @@
1applications:
2 ubuntu:
3 charm: ubuntu
4 num_units: 1
5relations:
6 - [ "ubuntu", "local-users" ]
diff --git a/tests/functional/tests/bundles/bionic.yaml b/tests/functional/tests/bundles/bionic.yaml
0new file mode 1200007new file mode 120000
index 0000000..f81f6ff
--- /dev/null
+++ b/tests/functional/tests/bundles/bionic.yaml
@@ -0,0 +1 @@
1base.yaml
0\ No newline at end of file2\ No newline at end of file
diff --git a/tests/functional/tests/bundles/focal.yaml b/tests/functional/tests/bundles/focal.yaml
1new file mode 1200003new file mode 120000
index 0000000..f81f6ff
--- /dev/null
+++ b/tests/functional/tests/bundles/focal.yaml
@@ -0,0 +1 @@
1base.yaml
0\ No newline at end of file2\ No newline at end of file
diff --git a/tests/functional/tests/bundles/jammy.yaml b/tests/functional/tests/bundles/jammy.yaml
1new file mode 1200003new file mode 120000
index 0000000..f81f6ff
--- /dev/null
+++ b/tests/functional/tests/bundles/jammy.yaml
@@ -0,0 +1 @@
1base.yaml
0\ No newline at end of file2\ No newline at end of file
diff --git a/tests/functional/tests/bundles/overlays/bionic.yaml.j2 b/tests/functional/tests/bundles/overlays/bionic.yaml.j2
1new file mode 1006443new file mode 100644
index 0000000..04930d7
--- /dev/null
+++ b/tests/functional/tests/bundles/overlays/bionic.yaml.j2
@@ -0,0 +1 @@
1series: bionic
0\ No newline at end of file2\ No newline at end of file
diff --git a/tests/functional/tests/bundles/overlays/focal.yaml.j2 b/tests/functional/tests/bundles/overlays/focal.yaml.j2
1new file mode 1006443new file mode 100644
index 0000000..1bdf16f
--- /dev/null
+++ b/tests/functional/tests/bundles/overlays/focal.yaml.j2
@@ -0,0 +1 @@
1series: focal
0\ No newline at end of file2\ No newline at end of file
diff --git a/tests/functional/tests/bundles/overlays/jammy.yaml.j2 b/tests/functional/tests/bundles/overlays/jammy.yaml.j2
1new file mode 1006443new file mode 100644
index 0000000..61d74ad
--- /dev/null
+++ b/tests/functional/tests/bundles/overlays/jammy.yaml.j2
@@ -0,0 +1 @@
1series: jammy
0\ No newline at end of file2\ No newline at end of file
diff --git a/tests/functional/tests/bundles/overlays/local-charm-overlay.yaml.j2 b/tests/functional/tests/bundles/overlays/local-charm-overlay.yaml.j2
1new file mode 1006443new file mode 100644
index 0000000..45065bc
--- /dev/null
+++ b/tests/functional/tests/bundles/overlays/local-charm-overlay.yaml.j2
@@ -0,0 +1,3 @@
1applications:
2 {{ charm_name }}:
3 charm: "{{ CHARM_LOCATION }}/{{ charm_name }}.charm"
diff --git a/tests/functional/tests/modules/__init__.py b/tests/functional/tests/modules/__init__.py
0new file mode 1006444new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/tests/functional/tests/modules/__init__.py
diff --git a/tests/functional/tests/modules/utils.py b/tests/functional/tests/modules/utils.py
1new file mode 1006445new file mode 100644
index 0000000..d1fd713
--- /dev/null
+++ b/tests/functional/tests/modules/utils.py
@@ -0,0 +1,37 @@
1"""Utilities to be used during charm-local-users functests."""
2import logging
3
4from cryptography.hazmat.primitives.asymmetric import rsa
5from cryptography.hazmat.primitives import serialization
6
7logger = logging.getLogger(__name__)
8
9
10def generate_keypair():
11 """Generate a public/private keypair."""
12 private_key = rsa.generate_private_key(public_exponent=65537, key_size=2048)
13 public_key = private_key.public_key()
14
15 private_key_string = (
16 private_key.private_bytes(
17 encoding=serialization.Encoding.PEM,
18 format=serialization.PrivateFormat.TraditionalOpenSSL,
19 encryption_algorithm=serialization.NoEncryption(),
20 )
21 .decode()
22 .strip()
23 )
24
25 public_key_string = (
26 public_key.public_bytes(
27 encoding=serialization.Encoding.OpenSSH,
28 format=serialization.PublicFormat.OpenSSH,
29 )
30 .decode()
31 .strip()
32 )
33
34 logger.info(f"Generated public key:\n{public_key_string}")
35 logger.info(f"Generated private key:\n{private_key_string}")
36
37 return public_key_string, private_key_string
diff --git a/tests/functional/tests/test_local_users.py b/tests/functional/tests/test_local_users.py
0new file mode 10064438new file mode 100644
index 0000000..9ce090b
--- /dev/null
+++ b/tests/functional/tests/test_local_users.py
@@ -0,0 +1,89 @@
1"""Functional tests for charm-local-users."""
2from subprocess import check_output
3from tempfile import NamedTemporaryFile
4import unittest
5
6import zaza
7
8from tests.modules.utils import generate_keypair
9
10
11class TestLocalUsers(unittest.TestCase):
12 """Tests related to the local-users charm."""
13
14 @classmethod
15 def setUpClass(cls):
16 """Test setup."""
17 cls.app_name = "local-users"
18 cls.principal_app_name = "ubuntu"
19 cls.principal_unit_name = cls.principal_app_name + "/0"
20 cls.ssh_pub_key, cls.ssh_priv_key = generate_keypair()
21
22 def wait_for_application_states(self):
23 """Wait for application states are ready.
24
25 zaza.model.block_until_all_applications_idle() seems to not wait for
26 (config-changed) to settle. Causing a race.
27 """
28 zaza.model.wait_for_application_states(
29 states={
30 self.app_name: {
31 "workload-status": "active",
32 "workload-status-message-regex": "^$",
33 },
34 self.principal_app_name: {
35 "workload-status": "active",
36 "workload-status-message-regex": "^$",
37 },
38 },
39 )
40
41 def test_10_default_ssh_login(self):
42 """Test the default ssh login functionality.
43
44 The path of the authorized_keys file can be configured,
45 we want the default configuration to allow logins
46 via standard paths.
47 """
48
49 zaza.model.set_application_config(
50 self.app_name, {"users": f"testuser;Test User;{self.ssh_pub_key}"}
51 )
52 self.wait_for_application_states()
53
54 with NamedTemporaryFile(
55 mode="w"
56 ) as private_key_file, NamedTemporaryFile() as known_hosts:
57 private_key_file.write(self.ssh_priv_key)
58 private_key_file.flush()
59
60 cmd = [
61 "ssh",
62 "-o",
63 "StrictHostKeyChecking=no",
64 "-o",
65 f"UserKnownHostsFile={known_hosts.name}",
66 "-i",
67 private_key_file.name,
68 "-l",
69 "testuser",
70 zaza.model.get_app_ips(self.principal_app_name)[0],
71 "whoami",
72 ]
73 stdout = check_output(cmd).decode().strip()
74 self.assertEqual(stdout, "testuser")
75
76 def test_11_ssh_custom_authorized_keys_file(self):
77 """Test the ssh-authorized-keys config option."""
78 zaza.model.set_application_config(
79 self.app_name,
80 {"ssh-authorized-keys": "/etc/ssh/user-authorized-keys/$USER"},
81 )
82 self.wait_for_application_states()
83
84 expected_ssh_pub_key = self.ssh_pub_key + " # charm-local-users\n"
85 zaza.model.block_until_file_has_contents(
86 application_name=self.principal_app_name,
87 remote_file="/etc/ssh/user-authorized-keys/testuser",
88 expected_contents=expected_ssh_pub_key,
89 )
diff --git a/tests/functional/tests/tests.yaml b/tests/functional/tests/tests.yaml
0new file mode 10064490new file mode 100644
index 0000000..6f8af3e
--- /dev/null
+++ b/tests/functional/tests/tests.yaml
@@ -0,0 +1,16 @@
1charm_name: local-users
2gate_bundles:
3 - jammy
4 - focal
5 - bionic
6dev_bundles:
7 - jammy
8tests:
9 - tests.test_local_users.TestLocalUsers
10target_deploy_status:
11 local-users:
12 workload-status: active
13 workload-status-message-regex: "^$"
14 ubuntu:
15 workload-status: active
16 workload-status-message-regex: "^$"
0\ No newline at end of file17\ No newline at end of file
diff --git a/tests/unit/test_local_users.py b/tests/unit/test_local_users.py
index 6f587b5..1f6a545 100644
--- a/tests/unit/test_local_users.py
+++ b/tests/unit/test_local_users.py
@@ -13,18 +13,68 @@
13# limitations under the License.13# limitations under the License.
1414
15import os15import os
16import pwd
16import unittest17import unittest
17from collections import namedtuple18from collections import namedtuple
19from pathlib import Path
18from tempfile import TemporaryDirectory20from tempfile import TemporaryDirectory
19from unittest.mock import patch21from unittest.mock import call, patch
2022
21from lib import local_users23from lib import local_users
2224
2325
24class TestLocalUsers(unittest.TestCase):26class TestLocalUsers(unittest.TestCase):
27 @patch("pwd.getpwnam")
28 def test_substitute_path_vars_for_user(self, mock_getpwnam):
29 mock_getpwnam.return_value = pwd.struct_passwd(
30 ("testuser", "x", 99999, 99999, "Test User", "/home/testuser", "/bin/bash")
31 )
32 self.assertEqual(
33 local_users._substitute_path_vars_for_user(
34 path=Path("/etc/ssh/user-authorized-keys/$USER"), username="testuser"
35 ),
36 Path("/etc/ssh/user-authorized-keys/testuser"),
37 )
38 self.assertEqual(
39 local_users._substitute_path_vars_for_user(
40 path=Path("/etc/ssh/user-authorized-keys/$UID"), username="testuser"
41 ),
42 Path("/etc/ssh/user-authorized-keys/99999"),
43 )
44 self.assertEqual(
45 local_users._substitute_path_vars_for_user(
46 path=Path("$HOME/.ssh/authorized_keys"), username="testuser"
47 ),
48 Path("/home/testuser/.ssh/authorized_keys"),
49 )
50
51 @patch("pwd.getpwnam")
52 def test_path_under_user_home(self, mock_getpwnam):
53 mock_getpwnam.return_value = pwd.struct_passwd(
54 ("testuser", "x", 99999, 99999, "Test User", "/home/testuser", "/bin/bash")
55 )
56 self.assertTrue(
57 local_users._path_under_user_home(
58 path=Path("/home/testuser/.ssh"), username="testuser"
59 )
60 )
61 self.assertFalse(
62 local_users._path_under_user_home(
63 path=Path("/etc/ssh"), username="testuser"
64 )
65 )
66 self.assertFalse(
67 local_users._path_under_user_home(
68 path=Path("/var/home/testuser/.ssh"), username="testuser"
69 )
70 )
71
25 @patch("shutil.chown")72 @patch("shutil.chown")
26 @patch("os.chmod")73 @patch("pathlib.Path.chmod")
27 def test_set_ssh_authorized_keys_update(self, *args, **kwargs):74 @patch("pwd.getpwnam")
75 def test_set_ssh_authorized_keys_update(
76 self, mock_getpwnam, mock_chmod, *args, **kwargs
77 ):
28 testuser = local_users.User(78 testuser = local_users.User(
29 "testuser", ["Test User", "", "", "", ""], ["ssh-rsa ABC testuser@testhost"]79 "testuser", ["Test User", "", "", "", ""], ["ssh-rsa ABC testuser@testhost"]
30 )80 )
@@ -34,27 +84,73 @@ class TestLocalUsers(unittest.TestCase):
34 )84 )
3585
36 with TemporaryDirectory() as fake_home:86 with TemporaryDirectory() as fake_home:
87 mock_getpwnam.return_value = pwd.struct_passwd(
88 ("testuser", "x", 99999, 99999, "Test User", fake_home, "/bin/bash")
89 )
90
91 testfile_path = os.path.join(fake_home, ".ssh", "authorized_keys")
92 local_users.set_ssh_authorized_keys(testuser, "$HOME/.ssh/authorized_keys")
93 with open(testfile_path, "r") as f:
94 keys = f.readlines()
95 self.assertIn(
96 "ssh-rsa ABC testuser@testhost # charm-local-users\n", keys
97 )
98
99 mock_chmod.assert_has_calls(
100 [
101 call(mode=0o700), # .ssh directory
102 call(mode=0o600), # auth_keys file
103 ]
104 )
105 mock_chmod.reset_mock()
106
107 # update the key
108 local_users.set_ssh_authorized_keys(testuser2, "$HOME/.ssh/authorized_keys")
109 with open(testfile_path, "r") as f:
110 keys = f.readlines()
111 self.assertIn(
112 "ssh-rsa XYZ testuser@testhost # charm-local-users\n", keys
113 )
114 self.assertNotIn(
115 "ssh-rsa ABC testuser@testhost # charm-local-users\n", keys
116 )
117
118 mock_chmod.assert_has_calls([call(mode=0o700), call(mode=0o600)])
119
120 @patch("pathlib.Path.chmod")
121 @patch("pwd.getpwnam")
122 def test_set_ssh_authorized_keys_in_etc(
123 self, mock_getpwnam, mock_chmod, *args, **kwargs
124 ):
125 testuser = local_users.User(
126 "testuser", ["Test User", "", "", "", ""], ["ssh-rsa ABC testuser@testhost"]
127 )
128
129 with TemporaryDirectory() as fake_etc:
130 mock_getpwnam.return_value = pwd.struct_passwd(
131 (
132 "testuser",
133 "x",
134 99999,
135 99999,
136 "Test User",
137 "/home/testuser",
138 "/bin/bash",
139 )
140 )
141
37 testfile_path = os.path.join(142 testfile_path = os.path.join(
38 fake_home, "testuser", ".ssh", "authorized_keys"143 fake_etc, "ssh", "user-authorized-keys", "testuser"
144 )
145 local_users.set_ssh_authorized_keys(
146 testuser, f"{fake_etc}/ssh/user-authorized-keys/$USER"
39 )147 )
40 with patch("lib.local_users.HOME_DIR_PATH", fake_home):148 with open(testfile_path, "r") as f:
41 local_users.set_ssh_authorized_keys(testuser)149 keys = f.readlines()
42 with open(testfile_path, "r") as f:150 self.assertIn(
43 keys = f.readlines()151 "ssh-rsa ABC testuser@testhost # charm-local-users\n", keys
44 self.assertIn(152 )
45 "ssh-rsa ABC testuser@testhost # charm-local-users\n", keys153 mock_chmod.assert_called_once_with(mode=0o644)
46 )
47
48 # update the key
49 local_users.set_ssh_authorized_keys(testuser2)
50 with open(testfile_path, "r") as f:
51 keys = f.readlines()
52 self.assertIn(
53 "ssh-rsa XYZ testuser@testhost # charm-local-users\n", keys
54 )
55 self.assertNotIn(
56 "ssh-rsa ABC testuser@testhost # charm-local-users\n", keys
57 )
58154
59 def test_parse_gecos(self):155 def test_parse_gecos(self):
60 test_cases = [156 test_cases = [
diff --git a/tox.ini b/tox.ini
index 9dc2441..0b80e54 100644
--- a/tox.ini
+++ b/tox.ini
@@ -25,8 +25,8 @@ commands = charmcraft build
2525
26[testenv:lint]26[testenv:lint]
27commands =27commands =
28 flake8 {posargs} src tests lib28 flake8 --config {toxinidir}/.flake8 {posargs} src tests lib
29 black --check --exclude "/(\.eggs|\.git|\.tox|\.venv|\.build|build|dist|charmhelpers|mod)/" .29 black --check --diff --color --exclude "/(\.eggs|\.git|\.tox|\.venv|\.build|build|dist|charmhelpers|mod)/" .
30deps =30deps =
31 black31 black
32 flake832 flake8
@@ -43,8 +43,8 @@ deps =
4343
44[testenv:unit]44[testenv:unit]
45commands =45commands =
46 coverage run --source=src -m unittest discover -s {toxinidir}/tests/unit -v46 coverage run --source=src,lib -m unittest discover -s {toxinidir}/tests/unit -v
47 coverage report --omit tests/*,mod/*,.tox/*47 coverage report -m --omit tests/*,mod/*,.tox/*
48 coverage html --omit tests/*,mod/*,.tox/*48 coverage html --omit tests/*,mod/*,.tox/*
49deps = -r{toxinidir}/tests/unit/requirements.txt49deps = -r{toxinidir}/tests/unit/requirements.txt
50 -r{toxinidir}/requirements.txt50 -r{toxinidir}/requirements.txt

Subscribers

People subscribed via source and target branches

to all changes: