Merge ~wck0/landscape-charm:set-secret-token into landscape-charm:main

[Bill Kronholm]

Add secret_token to the config.

[Bill Kronholm (wck0)]

Manual Testing Instructions

1. make build

2. Wait for the charm to settle and check the secret token is set.
juju ssh landscape-server/0
sudo less /etc/landscape/service.conf

3. Log in to the UI, click around, and check you can access the Organization page and the new beta UI.

4. Change the secret token:
juju config landscape-server secret_token="itsasecrettoeverybody"

5. Wait for the charm to settle and check the value was changed in service.conf.

6. Click around the UI and see that you are logged out of the old and new UIs.

7. Log back in and check things still work.

8. Change the secret token again:
juju config landscape-server secret_token=""

9. Wait for the charm to settle and check the value was changed in service.conf to something random.

10. Try to break something.

[Kevin Nasto (silverdrake11)]

I see the generate_secret_token code is from Landscape server. It's pretty outdated though. The following is from here https://docs.python.org/3/library/secrets.html and I think would be better. Note there are only 52 so not sure why the original code has [0:52] there.

import string
import secrets
alphabet = string.ascii_letters + string.digits
password = ''.join(secrets.choice(alphabet) for i in range(172))

[Kevin Nasto (silverdrake11)]

Regarding step 8, this is a bit undefined behavior. I'm not sure what it should do. One one hand setting it to empty will break it, on the other hand setting something to blank and then something being there doesn't make sense to me.

I would suggest to only on_install to generate it. And on_config change let the user do w/e

[Bill Kronholm (wck0)]

I need a sanity check on this.

With these updates, all units get the same secret-token set in the service.conf file.
However, I can't log in to the new UI as I get 401 {"message": "Auth token invalid. Please log in again.", "error": "AuthTokenInvalid"}

The login process is definitely getting and setting a JWT, and subsequent requests from the new UI set the appropriate request header.

--

To test:
Set num_units: 3 (or something bigger than 1 anyways) in the landscape-server section of the bundle.yaml file.
make build
Poke around in the browser.

[Kevin Nasto (silverdrake11)]

So 1 works but 2 doesn't?

[Bill Kronholm (wck0)]

> So 1 works but 2 doesn't?

I can't even log in to the new UI with only 1 landscape-server unit. Maybe it's an orthogonal issue.

5d9eb87... by Bill Kronholm on 2024-02-08

Use `secrets`

[Kevin Nasto (silverdrake11)]

When running my tests, I actually removed your changes in the _leader_elected function. This is because the token shouldn't change when a new leader is elected unlike the other stuff in that function that only runs on the leader. Despite removing all the units one by one and readding them, everything works

[Kevin Nasto (silverdrake11)]

review: Needs Fixing

ba26c36... by Bill Kronholm on 2024-02-08

DRY

[Bill Kronholm (wck0)]

Updated.

cb5e0aa... by Bill Kronholm on 2024-02-10

Write secret only if we need to.

d1700d4... by Bill Kronholm on 2024-02-10

Update help text.

[Bill Kronholm (wck0)]

Updated again.

[Kevin Nasto (silverdrake11)]

I tried to take down the leader so the others finish first, I thought it would not write the secret. But the others are waiting for the leader, then config-changed is triggered again. Anyway LGTM

review: Approve

diff --git a/config.yaml b/config.yaml
index 119b2aa..e6aa062 100644
--- a/config.yaml
+++ b/config.yaml
@@ -187,3 +187,9 @@ options:
     description: |
       Additional service.conf settings to be merged with the default
       configuration.
+  secret_token:
+    type: string
+    default:
+    description: |
+      A secret token for the landscape service. If not set one will be
+      generated securely.
diff --git a/src/charm.py b/src/charm.py
index 569201f..eb3a64d 100755
--- a/src/charm.py
+++ b/src/charm.py
@@ -49,8 +49,10 @@ from ops.model import (
 from settings_files import (
     DEFAULT_POSTGRES_PORT,
     configure_for_deployment_mode,
+    generate_secret_token,
     merge_service_conf,
     prepend_default_settings,
+    SecretTokenMissing,
     update_db_conf,
     update_default_settings,
     update_service_conf,
@@ -175,6 +177,7 @@ class LandscapeServerCharm(CharmBase):
         self._stored.set_default(paused=False)
         self._stored.set_default(default_root_url="")
         self._stored.set_default(account_bootstrapped=False)
+        self._stored.set_default(secret_token=None)
 
         self.landscape_uid = user_exists("landscape").pw_uid
         self.root_gid = group_exists("root").gr_gid
@@ -263,11 +266,36 @@ class LandscapeServerCharm(CharmBase):
 
         self._bootstrap_account()
 
+        secret_token = self._get_secret_token()
+        if self.unit.is_leader():
+            if not secret_token:
+                # If the secret token wasn't in the config, and we don't have one
+                # in the peer relation data, then the leader needs to generate one
+                # for all of the units to use.
+                logger.info("Generating new random secret token")
+                secret_token = generate_secret_token()
+                peer_relation = self.model.get_relation("replicas")
+                peer_relation.data[self.app].update({"secret-token": secret_token})
+        if (secret_token) and (secret_token != self._stored.secret_token):
+            self._write_secret_token(secret_token)
+            self._stored.secret_token = secret_token
+
         if isinstance(prev_status, BlockedStatus):
             self.unit.status = prev_status
 
         self._update_ready_status(restart_services=True)
 
+    def _get_secret_token(self):
+        secret_token = self.model.config.get("secret_token")
+        if not secret_token:
+            peer_relation = self.model.get_relation("replicas")
+            secret_token = peer_relation.data[self.app].get("secret-token", None)
+        return secret_token
+
+    def _write_secret_token(self, secret_token):
+        logger.info("Writing secret token")
+        update_service_conf({"landscape": {"secret-token": secret_token}})
+
     def _on_install(self, event: InstallEvent) -> None:
         """Handle the install event."""
         self.unit.status = MaintenanceStatus("Installing apt packages")
@@ -658,7 +686,7 @@ class LandscapeServerCharm(CharmBase):
         self.unit.status = MaintenanceStatus("Configuring HAProxy")
         haproxy_ssl_cert = event.relation.data[event.unit]["ssl_cert"]
 
-        # Sometimes the data has not been encoded propery in the HA charm
+        # Sometimes the data has not been encoded properly in the HA charm
         if haproxy_ssl_cert.startswith("b'"):
             haproxy_ssl_cert = haproxy_ssl_cert.strip('b').strip("'")
 
@@ -847,6 +875,12 @@ command[check_{service}]=/usr/local/lib/nagios/plugins/check_systemd.py {service
             for relation in haproxy_relations:
                 self._update_haproxy_connection(relation)
 
+        secret_token = self._get_secret_token()
+        if (secret_token) and (secret_token != self._stored.secret_token):
+            self._write_secret_token(secret_token)
+            self._stored.secret_token = secret_token
+            self._update_ready_status(restart_services=True)
+
     def _configure_smtp(self, relay_host: str) -> None:
 
         # Rewrite postfix config.
diff --git a/src/settings_files.py b/src/settings_files.py
index ba8211d..b4d1189 100644
--- a/src/settings_files.py
+++ b/src/settings_files.py
@@ -9,6 +9,8 @@ import os
 from base64 import b64decode, binascii
 from collections import defaultdict
 from configparser import ConfigParser
+import secrets
+from string import ascii_letters, digits
 from urllib.request import urlopen
 from urllib.error import URLError
 
@@ -41,6 +43,10 @@ class ServiceConfMissing(Exception):
     pass
 
 
+class SecretTokenMissing(Exception):
+    pass
+
+
 def configure_for_deployment_mode(mode: str) -> None:
     """
     Places files where Landscape expects to find them for different deployment
@@ -134,6 +140,11 @@ def update_service_conf(updates: dict) -> None:
         config.write(config_fp)
 
 
+def generate_secret_token():
+    alphanumerics = ascii_letters + digits
+    return "".join(secrets.choice(alphanumerics) for _ in range(172))
+
+
 def write_license_file(license_file: str, uid: int, gid: int) -> None:
     """
     Reads or decodes `license_file` to LICENSE_FILE and sets it up