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

[Mert Kirpici]

Close LP #1983437

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.

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

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

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:b6695bb22c939b70baf29c113ec9d009aa5fbdaa
https://jenkins.canonical.com/bootstack/job/lp-charm-local-users-ci/3/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-unit/295/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/1335/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-local-users-ci/3//rebuild

review: Needs Fixing (continuous-integration)

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

PASSED: Continuous integration, rev:e82f5cc0ee0845626947f47a851191d9fd05a35c
https://jenkins.canonical.com/bootstack/job/lp-charm-local-users-ci/4/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-unit/296/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/1336/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-local-users-ci/4//rebuild

review: Approve (continuous-integration)

[Mert Kirpici (mertkirpici)]

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.

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

PASSED: Continuous integration, rev:e82f5cc0ee0845626947f47a851191d9fd05a35c
https://jenkins.canonical.com/bootstack/job/lp-charm-local-users-ci/5/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/916/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/1341/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-local-users-ci/5//rebuild

review: Approve (continuous-integration)

[Gabriel Cocenza (gabrielcocenza)]

review: Needs Information

[Mert Kirpici (mertkirpici)]

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

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:a20398312c5feac33406a5b8faf221f2058e28e6
https://jenkins.canonical.com/bootstack/job/lp-charm-local-users-ci/6/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/992/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/1411/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-local-users-ci/6//rebuild

review: Needs Fixing (continuous-integration)

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

PASSED: Continuous integration, rev:a20398312c5feac33406a5b8faf221f2058e28e6
https://jenkins.canonical.com/bootstack/job/lp-charm-local-users-ci/7/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/993/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/1412/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-local-users-ci/7//rebuild

review: Approve (continuous-integration)

[Gabriel Cocenza (gabrielcocenza)]

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.

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

PASSED: Continuous integration, rev:1a33ac7031a85ee381794c8e2cb712edc2411e1b
https://jenkins.canonical.com/bootstack/job/lp-charm-local-users-ci/8/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/1006/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/1421/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-local-users-ci/8//rebuild

review: Approve (continuous-integration)

[Mert Kirpici (mertkirpici)]

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

[Gabriel Cocenza (gabrielcocenza)]

Thanks Mert. LGTM

Lefty two nit comments in the unit tests

review: Approve

[Eric Chen (eric-chen)]

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

review: Needs Fixing

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

PASSED: Continuous integration, rev:9f62485be1332d2359303e1f1565d9b5e38e1bc3
https://jenkins.canonical.com/bootstack/job/lp-charm-local-users-ci/9/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/1033/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/1444/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-local-users-ci/9//rebuild

review: Approve (continuous-integration)

[Mert Kirpici (mertkirpici)]

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

[Eric Chen (eric-chen)]

review: Approve

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision fdc2f14b7406486ad805917e45a965547a6166b5

diff --git a/Makefile b/Makefile
index f335696..57a3fad 100644
--- a/Makefile
+++ b/Makefile
@@ -82,11 +82,7 @@ unittests:
 	@echo "Running unit tests"
 	@tox -e unit
 
-snap:
-	@echo "Downloading snap"
-	snap download juju-lint --basename juju-lint --target-directory tests/functional/tests/resources
-
-functional: build snap
+functional: build
 	@echo "Executing functional tests in ${CHARM_BUILD_DIR}"
 	@CHARM_LOCATION=${PROJECTPATH} tox -e func
 
diff --git a/config.yaml b/config.yaml
index 2a7920c..3f5d8dd 100644
--- a/config.yaml
+++ b/config.yaml
@@ -30,6 +30,13 @@ options:
     description:
       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.
       Setting to True disables that protection and allows for pre-existing users to be added to the charm managed group.
+  ssh-authorized-keys:
+    default: "$HOME/.ssh/authorized_keys"
+    type: string
+    description: |
+      The file to write the SSH public keys to.
+      This option supports the usage of variables "$USER", "$HOME" and "$UID" in the path string.
+      They will be expanded to the username, home directory and the user id of each user.
   sudoers:
     default: ""
     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 @@
 import grp
 import logging
 import os
+import pwd
 import re
 import shutil
 import subprocess
 import tempfile
 from collections import namedtuple
+from pathlib import Path
+from typing import Text
 
 from charmhelpers.core import host
 
@@ -29,7 +32,6 @@ log = logging.getLogger(__name__)
 
 User = namedtuple("User", ["name", "gecos", "ssh_keys"])
 
-HOME_DIR_PATH = "/home"
 SUDOERS_FILE = "/etc/sudoers.d/70-local-users-charm"
 
 
@@ -48,7 +50,7 @@ def add_user(username, shell="/bin/bash", home_dir=None, gecos=None):
     subprocess.check_call(cmd)
 
 
-def configure_user(user, group):
+def configure_user(user, group, authorized_keys_path):
     """Idempotently apply requested User configuration.
 
     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):
             gecos=user.gecos,
         )
     host.add_user_to_group(user.name, group)
-    set_ssh_authorized_keys(user)
+    set_ssh_authorized_keys(user, authorized_keys_path)
     update_gecos(user)
 
 
@@ -121,7 +123,27 @@ def rename_group(old_name, new_name):
         subprocess.check_call(cmd)
 
 
-def set_ssh_authorized_keys(user):
+def _substitute_path_vars_for_user(path: Path, username: Text) -> Path:
+    """Substitute common variables in terms of a user.
+
+    Supports the variables $HOME, $USER and $UID.
+    """
+    passwd_entry = pwd.getpwnam(username)
+    return Path(
+        str(path)
+        .replace("$HOME", passwd_entry.pw_dir)
+        .replace("$USER", passwd_entry.pw_name)
+        .replace("$UID", str(passwd_entry.pw_uid))
+    )
+
+
+def _path_under_user_home(path: Path, username: Text) -> bool:
+    """Test whether the given path lies inside the user home."""
+    passwd_entry = pwd.getpwnam(username)
+    return str(path).startswith(passwd_entry.pw_dir + "/")
+
+
+def set_ssh_authorized_keys(user, authorized_keys_path):
     """Idempotently set up the SSH public key in `authorized_keys`."""
     comment = "# charm-local-users"
     authorized_keys = []
@@ -129,21 +151,24 @@ def set_ssh_authorized_keys(user):
     for ssh_key in user.ssh_keys:
         authorized_key = " ".join([ssh_key, comment])
         authorized_keys.append(authorized_key)
-    ssh_path = os.path.join(HOME_DIR_PATH, user.name, ".ssh")
-    authorized_keys_path = os.path.join(ssh_path, "authorized_keys")
 
-    if not os.path.exists(ssh_path):
-        os.makedirs(ssh_path, mode=0o700)
-    os.chmod(ssh_path, 0o700)
-    shutil.chown(ssh_path, user=user.name, group=user.name)
+    authorized_keys_path = _substitute_path_vars_for_user(
+        Path(authorized_keys_path), user.name
+    )
+    ssh_path = authorized_keys_path.parent
+    ssh_path_under_user_home = _path_under_user_home(ssh_path, user.name)
+
+    ssh_path.mkdir(parents=True, exist_ok=True)
+
+    if ssh_path_under_user_home:
+        ssh_path.chmod(mode=0o700)
+        shutil.chown(ssh_path, user=user.name, group=user.name)
 
     # get currently configured keys
     current_keys = []
 
-    if os.path.exists(authorized_keys_path):
-        with open(authorized_keys_path, "r") as keys_file:
-            current_keys = keys_file.readlines()
-            keys_file.close()
+    if authorized_keys_path.exists():
+        current_keys = authorized_keys_path.read_text().splitlines()
 
     # keep the non-managed keys
     regex = re.compile(r"charm-local-users")
@@ -152,16 +177,18 @@ def set_ssh_authorized_keys(user):
 
     for authorized_key in authorized_keys:
         new_keys.append(authorized_key + "\n")
-    with open(authorized_keys_path, "w+") as keys_file:
+    with authorized_keys_path.open("w+") as keys_file:
         for key in new_keys:
             keys_file.write(key)
-        keys_file.close()
 
     # ensure correct permissions
 
-    if os.path.exists(authorized_keys_path):
-        os.chmod(authorized_keys_path, 0o600)
-        shutil.chown(authorized_keys_path, user=user.name, group=user.name)
+    if authorized_keys_path.exists():
+        if ssh_path_under_user_home:
+            authorized_keys_path.chmod(mode=0o600)
+            shutil.chown(authorized_keys_path, user=user.name, group=user.name)
+        else:
+            authorized_keys_path.chmod(mode=0o644)
 
 
 def 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):
                 delete_user(u, backup_path)
 
         # configure user accounts specified in the config
+        authorized_keys_path = self.config["ssh-authorized-keys"]
         for user in userlist:
-            configure_user(user, group)
+            configure_user(user, group, authorized_keys_path)
 
         # Configure custom /etc/sudoers.d file
         sudoers = self.config["sudoers"]
diff --git a/tests/functional/requirements.txt b/tests/functional/requirements.txt
new file mode 100644
index 0000000..ea401e0
--- /dev/null
+++ b/tests/functional/requirements.txt
@@ -0,0 +1,3 @@
+git+https://github.com/openstack-charmers/zaza.git#egg=zaza
+cryptography
+python-openstackclient
\ No newline at end of file
diff --git a/tests/functional/tests/bundles/base.yaml b/tests/functional/tests/bundles/base.yaml
new file mode 100644
index 0000000..86e83a1
--- /dev/null
+++ b/tests/functional/tests/bundles/base.yaml
@@ -0,0 +1,6 @@
+applications:
+  ubuntu:
+    charm: ubuntu
+    num_units: 1
+relations:
+  - [ "ubuntu", "local-users" ]
diff --git a/tests/functional/tests/bundles/bionic.yaml b/tests/functional/tests/bundles/bionic.yaml
new file mode 120000
index 0000000..f81f6ff
--- /dev/null
+++ b/tests/functional/tests/bundles/bionic.yaml
@@ -0,0 +1 @@
+base.yaml
\ No newline at end of file
diff --git a/tests/functional/tests/bundles/focal.yaml b/tests/functional/tests/bundles/focal.yaml
new file mode 120000
index 0000000..f81f6ff
--- /dev/null
+++ b/tests/functional/tests/bundles/focal.yaml
@@ -0,0 +1 @@
+base.yaml
\ No newline at end of file
diff --git a/tests/functional/tests/bundles/jammy.yaml b/tests/functional/tests/bundles/jammy.yaml
new file mode 120000
index 0000000..f81f6ff
--- /dev/null
+++ b/tests/functional/tests/bundles/jammy.yaml
@@ -0,0 +1 @@
+base.yaml
\ 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
new file mode 100644
index 0000000..04930d7
--- /dev/null
+++ b/tests/functional/tests/bundles/overlays/bionic.yaml.j2
@@ -0,0 +1 @@
+series: bionic
\ 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
new file mode 100644
index 0000000..1bdf16f
--- /dev/null
+++ b/tests/functional/tests/bundles/overlays/focal.yaml.j2
@@ -0,0 +1 @@
+series: focal
\ 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
new file mode 100644
index 0000000..61d74ad
--- /dev/null
+++ b/tests/functional/tests/bundles/overlays/jammy.yaml.j2
@@ -0,0 +1 @@
+series: jammy
\ 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
new file mode 100644
index 0000000..45065bc
--- /dev/null
+++ b/tests/functional/tests/bundles/overlays/local-charm-overlay.yaml.j2
@@ -0,0 +1,3 @@
+applications:
+  {{ charm_name }}:
+    charm: "{{ CHARM_LOCATION }}/{{ charm_name }}.charm"
diff --git a/tests/functional/tests/modules/__init__.py b/tests/functional/tests/modules/__init__.py
new 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
new file mode 100644
index 0000000..d1fd713
--- /dev/null
+++ b/tests/functional/tests/modules/utils.py
@@ -0,0 +1,37 @@
+"""Utilities to be used during charm-local-users functests."""
+import logging
+
+from cryptography.hazmat.primitives.asymmetric import rsa
+from cryptography.hazmat.primitives import serialization
+
+logger = logging.getLogger(__name__)
+
+
+def generate_keypair():
+    """Generate a public/private keypair."""
+    private_key = rsa.generate_private_key(public_exponent=65537, key_size=2048)
+    public_key = private_key.public_key()
+
+    private_key_string = (
+        private_key.private_bytes(
+            encoding=serialization.Encoding.PEM,
+            format=serialization.PrivateFormat.TraditionalOpenSSL,
+            encryption_algorithm=serialization.NoEncryption(),
+        )
+        .decode()
+        .strip()
+    )
+
+    public_key_string = (
+        public_key.public_bytes(
+            encoding=serialization.Encoding.OpenSSH,
+            format=serialization.PublicFormat.OpenSSH,
+        )
+        .decode()
+        .strip()
+    )
+
+    logger.info(f"Generated public key:\n{public_key_string}")
+    logger.info(f"Generated private key:\n{private_key_string}")
+
+    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
new file mode 100644
index 0000000..9ce090b
--- /dev/null
+++ b/tests/functional/tests/test_local_users.py
@@ -0,0 +1,89 @@
+"""Functional tests for charm-local-users."""
+from subprocess import check_output
+from tempfile import NamedTemporaryFile
+import unittest
+
+import zaza
+
+from tests.modules.utils import generate_keypair
+
+
+class TestLocalUsers(unittest.TestCase):
+    """Tests related to the local-users charm."""
+
+    @classmethod
+    def setUpClass(cls):
+        """Test setup."""
+        cls.app_name = "local-users"
+        cls.principal_app_name = "ubuntu"
+        cls.principal_unit_name = cls.principal_app_name + "/0"
+        cls.ssh_pub_key, cls.ssh_priv_key = generate_keypair()
+
+    def wait_for_application_states(self):
+        """Wait for application states are ready.
+
+        zaza.model.block_until_all_applications_idle() seems to not wait for
+        (config-changed) to settle. Causing a race.
+        """
+        zaza.model.wait_for_application_states(
+            states={
+                self.app_name: {
+                    "workload-status": "active",
+                    "workload-status-message-regex": "^$",
+                },
+                self.principal_app_name: {
+                    "workload-status": "active",
+                    "workload-status-message-regex": "^$",
+                },
+            },
+        )
+
+    def test_10_default_ssh_login(self):
+        """Test the default ssh login functionality.
+
+        The path of the authorized_keys file can be configured,
+        we want the default configuration to allow logins
+        via standard paths.
+        """
+
+        zaza.model.set_application_config(
+            self.app_name, {"users": f"testuser;Test User;{self.ssh_pub_key}"}
+        )
+        self.wait_for_application_states()
+
+        with NamedTemporaryFile(
+            mode="w"
+        ) as private_key_file, NamedTemporaryFile() as known_hosts:
+            private_key_file.write(self.ssh_priv_key)
+            private_key_file.flush()
+
+            cmd = [
+                "ssh",
+                "-o",
+                "StrictHostKeyChecking=no",
+                "-o",
+                f"UserKnownHostsFile={known_hosts.name}",
+                "-i",
+                private_key_file.name,
+                "-l",
+                "testuser",
+                zaza.model.get_app_ips(self.principal_app_name)[0],
+                "whoami",
+            ]
+            stdout = check_output(cmd).decode().strip()
+        self.assertEqual(stdout, "testuser")
+
+    def test_11_ssh_custom_authorized_keys_file(self):
+        """Test the ssh-authorized-keys config option."""
+        zaza.model.set_application_config(
+            self.app_name,
+            {"ssh-authorized-keys": "/etc/ssh/user-authorized-keys/$USER"},
+        )
+        self.wait_for_application_states()
+
+        expected_ssh_pub_key = self.ssh_pub_key + " # charm-local-users\n"
+        zaza.model.block_until_file_has_contents(
+            application_name=self.principal_app_name,
+            remote_file="/etc/ssh/user-authorized-keys/testuser",
+            expected_contents=expected_ssh_pub_key,
+        )
diff --git a/tests/functional/tests/tests.yaml b/tests/functional/tests/tests.yaml
new file mode 100644
index 0000000..6f8af3e
--- /dev/null
+++ b/tests/functional/tests/tests.yaml
@@ -0,0 +1,16 @@
+charm_name: local-users
+gate_bundles:
+  - jammy
+  - focal
+  - bionic
+dev_bundles:
+  - jammy
+tests:
+  - tests.test_local_users.TestLocalUsers
+target_deploy_status:
+  local-users:
+    workload-status: active
+    workload-status-message-regex: "^$"
+  ubuntu:
+    workload-status: active
+    workload-status-message-regex: "^$"
\ 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 @@
 # limitations under the License.
 
 import os
+import pwd
 import unittest
 from collections import namedtuple
+from pathlib import Path
 from tempfile import TemporaryDirectory
-from unittest.mock import patch
+from unittest.mock import call, patch
 
 from lib import local_users
 
 
 class TestLocalUsers(unittest.TestCase):
+    @patch("pwd.getpwnam")
+    def test_substitute_path_vars_for_user(self, mock_getpwnam):
+        mock_getpwnam.return_value = pwd.struct_passwd(
+            ("testuser", "x", 99999, 99999, "Test User", "/home/testuser", "/bin/bash")
+        )
+        self.assertEqual(
+            local_users._substitute_path_vars_for_user(
+                path=Path("/etc/ssh/user-authorized-keys/$USER"), username="testuser"
+            ),
+            Path("/etc/ssh/user-authorized-keys/testuser"),
+        )
+        self.assertEqual(
+            local_users._substitute_path_vars_for_user(
+                path=Path("/etc/ssh/user-authorized-keys/$UID"), username="testuser"
+            ),
+            Path("/etc/ssh/user-authorized-keys/99999"),
+        )
+        self.assertEqual(
+            local_users._substitute_path_vars_for_user(
+                path=Path("$HOME/.ssh/authorized_keys"), username="testuser"
+            ),
+            Path("/home/testuser/.ssh/authorized_keys"),
+        )
+
+    @patch("pwd.getpwnam")
+    def test_path_under_user_home(self, mock_getpwnam):
+        mock_getpwnam.return_value = pwd.struct_passwd(
+            ("testuser", "x", 99999, 99999, "Test User", "/home/testuser", "/bin/bash")
+        )
+        self.assertTrue(
+            local_users._path_under_user_home(
+                path=Path("/home/testuser/.ssh"), username="testuser"
+            )
+        )
+        self.assertFalse(
+            local_users._path_under_user_home(
+                path=Path("/etc/ssh"), username="testuser"
+            )
+        )
+        self.assertFalse(
+            local_users._path_under_user_home(
+                path=Path("/var/home/testuser/.ssh"), username="testuser"
+            )
+        )
+
     @patch("shutil.chown")
-    @patch("os.chmod")
-    def test_set_ssh_authorized_keys_update(self, *args, **kwargs):
+    @patch("pathlib.Path.chmod")
+    @patch("pwd.getpwnam")
+    def test_set_ssh_authorized_keys_update(
+        self, mock_getpwnam, mock_chmod, *args, **kwargs
+    ):
         testuser = local_users.User(
             "testuser", ["Test User", "", "", "", ""], ["ssh-rsa ABC testuser@testhost"]
         )
@@ -34,27 +84,73 @@ class TestLocalUsers(unittest.TestCase):
         )
 
         with TemporaryDirectory() as fake_home:
+            mock_getpwnam.return_value = pwd.struct_passwd(
+                ("testuser", "x", 99999, 99999, "Test User", fake_home, "/bin/bash")
+            )
+
+            testfile_path = os.path.join(fake_home, ".ssh", "authorized_keys")
+            local_users.set_ssh_authorized_keys(testuser, "$HOME/.ssh/authorized_keys")
+            with open(testfile_path, "r") as f:
+                keys = f.readlines()
+                self.assertIn(
+                    "ssh-rsa ABC testuser@testhost # charm-local-users\n", keys
+                )
+
+            mock_chmod.assert_has_calls(
+                [
+                    call(mode=0o700),  # .ssh directory
+                    call(mode=0o600),  # auth_keys file
+                ]
+            )
+            mock_chmod.reset_mock()
+
+            # update the key
+            local_users.set_ssh_authorized_keys(testuser2, "$HOME/.ssh/authorized_keys")
+            with open(testfile_path, "r") as f:
+                keys = f.readlines()
+                self.assertIn(
+                    "ssh-rsa XYZ testuser@testhost # charm-local-users\n", keys
+                )
+                self.assertNotIn(
+                    "ssh-rsa ABC testuser@testhost # charm-local-users\n", keys
+                )
+
+            mock_chmod.assert_has_calls([call(mode=0o700), call(mode=0o600)])
+
+    @patch("pathlib.Path.chmod")
+    @patch("pwd.getpwnam")
+    def test_set_ssh_authorized_keys_in_etc(
+        self, mock_getpwnam, mock_chmod, *args, **kwargs
+    ):
+        testuser = local_users.User(
+            "testuser", ["Test User", "", "", "", ""], ["ssh-rsa ABC testuser@testhost"]
+        )
+
+        with TemporaryDirectory() as fake_etc:
+            mock_getpwnam.return_value = pwd.struct_passwd(
+                (
+                    "testuser",
+                    "x",
+                    99999,
+                    99999,
+                    "Test User",
+                    "/home/testuser",
+                    "/bin/bash",
+                )
+            )
+
             testfile_path = os.path.join(
-                fake_home, "testuser", ".ssh", "authorized_keys"
+                fake_etc, "ssh", "user-authorized-keys", "testuser"
+            )
+            local_users.set_ssh_authorized_keys(
+                testuser, f"{fake_etc}/ssh/user-authorized-keys/$USER"
             )
-            with patch("lib.local_users.HOME_DIR_PATH", fake_home):
-                local_users.set_ssh_authorized_keys(testuser)
-                with open(testfile_path, "r") as f:
-                    keys = f.readlines()
-                    self.assertIn(
-                        "ssh-rsa ABC testuser@testhost # charm-local-users\n", keys
-                    )
-
-                # update the key
-                local_users.set_ssh_authorized_keys(testuser2)
-                with open(testfile_path, "r") as f:
-                    keys = f.readlines()
-                    self.assertIn(
-                        "ssh-rsa XYZ testuser@testhost # charm-local-users\n", keys
-                    )
-                    self.assertNotIn(
-                        "ssh-rsa ABC testuser@testhost # charm-local-users\n", keys
-                    )
+            with open(testfile_path, "r") as f:
+                keys = f.readlines()
+                self.assertIn(
+                    "ssh-rsa ABC testuser@testhost # charm-local-users\n", keys
+                )
+        mock_chmod.assert_called_once_with(mode=0o644)
 
     def test_parse_gecos(self):
         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
 
 [testenv:lint]
 commands =
-    flake8 {posargs} src tests lib
-    black --check --exclude "/(\.eggs|\.git|\.tox|\.venv|\.build|build|dist|charmhelpers|mod)/" .
+    flake8 --config {toxinidir}/.flake8 {posargs} src tests lib
+    black --check --diff --color --exclude "/(\.eggs|\.git|\.tox|\.venv|\.build|build|dist|charmhelpers|mod)/" .
 deps =
     black
     flake8
@@ -43,8 +43,8 @@ deps =
 
 [testenv:unit]
 commands =
-    coverage run --source=src -m unittest discover -s {toxinidir}/tests/unit -v
-    coverage report --omit tests/*,mod/*,.tox/*
+    coverage run --source=src,lib -m unittest discover -s {toxinidir}/tests/unit -v
+    coverage report -m --omit tests/*,mod/*,.tox/*
     coverage html --omit tests/*,mod/*,.tox/*
 deps = -r{toxinidir}/tests/unit/requirements.txt
        -r{toxinidir}/requirements.txt