Merge pull request #1029 from alesstimec/configurable-macaroon-expiry

CSS-5106 Makes macaroon expiry duration configurable.
canonical · Aug 22, 2023 · 57607e5 · 57607e5
2 parents 8139e52 + 48c07ea
commit 57607e5
Show file tree

Hide file tree

Showing 10 changed files with 64 additions and 21 deletions.
diff --git a/charms/jimm-k8s/config.yaml b/charms/jimm-k8s/config.yaml
@@ -57,3 +57,7 @@ options:
   dns-name:
     type: string
     description: DNS hostname that JIMM is being served from.
+  macaroon-expiry-duration:
+    type: string
+    default: 24h
+    description: Expiry duration for authentication macaroons.
diff --git a/charms/jimm-k8s/src/charm.py b/charms/jimm-k8s/src/charm.py
@@ -222,6 +222,7 @@ def _update_workload(self, event):
             "JIMM_UUID": self.config.get("uuid", ""),
             "JIMM_DASHBOARD_LOCATION": self.config.get("juju-dashboard-location", "https://jaas.ai/models"),
             "JIMM_LISTEN_ADDR": ":8080",
+            "JIMM_MACAROON_EXPIRY_DURATION": self.config.get("macaroon-expiry-duration", "24h"),
         }
         if self._state.dsn:
             config_values["JIMM_DSN"] = self._state.dsn

diff --git a/charms/jimm-k8s/tests/unit/test_charm.py b/charms/jimm-k8s/tests/unit/test_charm.py
@@ -17,6 +17,7 @@
 MINIMAL_CONFIG = {
     "uuid": "1234567890",
     "candid-url": "test-candid-url",
+    "macaroon-expiry-duration": "48h",
 }
 
 
@@ -72,6 +73,7 @@ def test_on_pebble_ready(self):
                             "JIMM_DNS_NAME": "juju-jimm-k8s-0.juju-jimm-k8s-endpoints.None.svc.cluster.local",
                             "JIMM_LISTEN_ADDR": ":8080",
                             "JIMM_LOG_LEVEL": "info",
+                            "JIMM_MACAROON_EXPIRY_DURATION": "48h",
                             "JIMM_UUID": "1234567890",
                             "JIMM_WATCH_CONTROLLERS": "1",
                         },
@@ -107,6 +109,7 @@ def test_on_config_changed(self):
                             "JIMM_DNS_NAME": "juju-jimm-k8s-0.juju-jimm-k8s-endpoints.None.svc.cluster.local",
                             "JIMM_LISTEN_ADDR": ":8080",
                             "JIMM_LOG_LEVEL": "info",
+                            "JIMM_MACAROON_EXPIRY_DURATION": "48h",
                             "JIMM_UUID": "1234567890",
                             "JIMM_WATCH_CONTROLLERS": "1",
                         },
@@ -150,6 +153,7 @@ def test_bakery_configuration(self):
                             "JIMM_DNS_NAME": "juju-jimm-k8s-0.juju-jimm-k8s-endpoints.None.svc.cluster.local",
                             "JIMM_LISTEN_ADDR": ":8080",
                             "JIMM_LOG_LEVEL": "info",
+                            "JIMM_MACAROON_EXPIRY_DURATION": "24h",
                             "JIMM_UUID": "1234567890",
                             "JIMM_WATCH_CONTROLLERS": "1",
                         },

diff --git a/charms/jimm/actions.yaml b/charms/jimm/actions.yaml
@@ -1,3 +1,2 @@
 # Copyright 2021 Canonical Ltd
 # See LICENSE file for licensing details.
-
diff --git a/charms/jimm/config.yaml b/charms/jimm/config.yaml
@@ -54,3 +54,7 @@ options:
     type: string
     default: https://jaas.ai/models
     description: URL of the Juju Dashboard for this controller.
+  macaroon-expiry-duration:
+    type: string
+    default: 24h
+    description: Expiry duration for authentication macaroons.
diff --git a/charms/jimm/src/charm.py b/charms/jimm/src/charm.py
@@ -122,6 +122,7 @@ def _on_config_changed(self, _):
             "log_level": self.config.get("log-level"),
             "uuid": self.config.get("uuid"),
             "dashboard_location": self.config.get("juju-dashboard-location"),
+            "macaroon_expiry_duration": self.config.get("macaroon-expiry-duration"),
         }
 
         with open(self._env_filename(), "wt") as f:

diff --git a/charms/jimm/templates/jimm.env b/charms/jimm/templates/jimm.env
@@ -9,3 +9,4 @@ JIMM_DNS_NAME={{dns_name}}
 {% endif %}
 JIMM_LOG_LEVEL={{log_level}}
 JIMM_UUID={{uuid}}
+JIMM_MACAROON_EXPIRY_DURATION={{macaroon_expiry_duration}}
diff --git a/charms/jimm/tests/test_charm.py b/charms/jimm/tests/test_charm.py
@@ -115,13 +115,14 @@ def test_config_changed(self):
                 "dns-name": "jimm.example.com",
                 "log-level": "debug",
                 "uuid": "caaa4ba4-e2b5-40dd-9bf3-2bd26d6e17aa",
+                "macaroon-expiry-duration": "48h",
             }
         )
         self.assertTrue(os.path.exists(config_file))
         with open(config_file) as f:
             lines = f.readlines()
         os.unlink(config_file)
-        self.assertEqual(len(lines), 11)
+        self.assertEqual(len(lines), 12)
         self.assertEqual(lines[0].strip(), "BAKERY_AGENT_FILE=")
         self.assertEqual(lines[1].strip(), "CANDID_URL=https://candid.example.com")
         self.assertEqual(lines[2].strip(), "JIMM_ADMINS=user1 user2 group1")
@@ -132,6 +133,7 @@ def test_config_changed(self):
         self.assertEqual(lines[7].strip(), "JIMM_DNS_NAME=" + "jimm.example.com")
         self.assertEqual(lines[9].strip(), "JIMM_LOG_LEVEL=debug")
         self.assertEqual(lines[10].strip(), "JIMM_UUID=caaa4ba4-e2b5-40dd-9bf3-2bd26d6e17aa")
+        self.assertEqual(lines[11].strip(), "JIMM_MACAROON_EXPIRY_DURATION=48h")
 
     def test_config_changed_redirect_to_dashboard(self):
         config_file = os.path.join(self.harness.charm.charm_dir, "juju-jimm.env")
@@ -143,13 +145,14 @@ def test_config_changed_redirect_to_dashboard(self):
                 "log-level": "debug",
                 "uuid": "caaa4ba4-e2b5-40dd-9bf3-2bd26d6e17aa",
                 "juju-dashboard-location": "https://test.jaas.ai/models",
+                "macaroon-expiry-duration": "48h",
             }
         )
         self.assertTrue(os.path.exists(config_file))
         with open(config_file) as f:
             lines = f.readlines()
         os.unlink(config_file)
-        self.assertEqual(len(lines), 11)
+        self.assertEqual(len(lines), 12)
         self.assertEqual(lines[0].strip(), "BAKERY_AGENT_FILE=")
         self.assertEqual(lines[1].strip(), "CANDID_URL=https://candid.example.com")
         self.assertEqual(lines[2].strip(), "JIMM_ADMINS=user1 user2 group1")
@@ -160,6 +163,7 @@ def test_config_changed_redirect_to_dashboard(self):
         self.assertEqual(lines[7].strip(), "JIMM_DNS_NAME=" + "jimm.example.com")
         self.assertEqual(lines[9].strip(), "JIMM_LOG_LEVEL=debug")
         self.assertEqual(lines[10].strip(), "JIMM_UUID=caaa4ba4-e2b5-40dd-9bf3-2bd26d6e17aa")
+        self.assertEqual(lines[11].strip(), "JIMM_MACAROON_EXPIRY_DURATION=48h")
 
     def test_config_changed_ready(self):
         config_file = os.path.join(self.harness.charm.charm_dir, "juju-jimm.env")
@@ -170,13 +174,14 @@ def test_config_changed_ready(self):
                 "candid-url": "https://candid.example.com",
                 "controller-admins": "user1 user2 group1",
                 "uuid": "caaa4ba4-e2b5-40dd-9bf3-2bd26d6e17aa",
+                "macaroon-expiry-duration": "48h",
             }
         )
         self.assertTrue(os.path.exists(config_file))
         with open(config_file) as f:
             lines = f.readlines()
         os.unlink(config_file)
-        self.assertEqual(len(lines), 9)
+        self.assertEqual(len(lines), 10)
         self.assertEqual(lines[0].strip(), "BAKERY_AGENT_FILE=")
         self.assertEqual(lines[1].strip(), "CANDID_URL=https://candid.example.com")
         self.assertEqual(lines[2].strip(), "JIMM_ADMINS=user1 user2 group1")
@@ -186,6 +191,7 @@ def test_config_changed_ready(self):
         )
         self.assertEqual(lines[7].strip(), "JIMM_LOG_LEVEL=info")
         self.assertEqual(lines[8].strip(), "JIMM_UUID=caaa4ba4-e2b5-40dd-9bf3-2bd26d6e17aa")
+        self.assertEqual(lines[9].strip(), "JIMM_MACAROON_EXPIRY_DURATION=48h")
 
     def test_config_changed_with_agent(self):
         config_file = os.path.join(self.harness.charm.charm_dir, "juju-jimm.env")
@@ -210,7 +216,7 @@ def test_config_changed_with_agent(self):
 
         with open(config_file) as f:
             lines = f.readlines()
-        self.assertEqual(len(lines), 9)
+        self.assertEqual(len(lines), 10)
         self.assertEqual(
             lines[0].strip(),
             "BAKERY_AGENT_FILE=" + self.harness.charm._agent_filename,
@@ -220,6 +226,7 @@ def test_config_changed_with_agent(self):
         self.assertEqual(lines[4].strip(), "JIMM_DASHBOARD_LOCATION=https://jaas.ai/models")
         self.assertEqual(lines[7].strip(), "JIMM_LOG_LEVEL=info")
         self.assertEqual(lines[8].strip(), "JIMM_UUID=caaa4ba4-e2b5-40dd-9bf3-2bd26d6e17aa")
+        self.assertEqual(lines[9].strip(), "JIMM_MACAROON_EXPIRY_DURATION=24h")
 
         self.harness.charm._agent_filename = os.path.join(self.tempdir.name, "no-such-dir", "agent.json")
         self.harness.update_config(
@@ -234,13 +241,14 @@ def test_config_changed_with_agent(self):
         )
         with open(config_file) as f:
             lines = f.readlines()
-        self.assertEqual(len(lines), 9)
+        self.assertEqual(len(lines), 10)
         self.assertEqual(lines[0].strip(), "BAKERY_AGENT_FILE=")
         self.assertEqual(lines[1].strip(), "CANDID_URL=https://candid.example.com")
         self.assertEqual(lines[2].strip(), "JIMM_ADMINS=user1 user2 group1")
         self.assertEqual(lines[4].strip(), "JIMM_DASHBOARD_LOCATION=https://jaas.ai/models")
         self.assertEqual(lines[7].strip(), "JIMM_LOG_LEVEL=info")
         self.assertEqual(lines[8].strip(), "JIMM_UUID=caaa4ba4-e2b5-40dd-9bf3-2bd26d6e17aa")
+        self.assertEqual(lines[9].strip(), "JIMM_MACAROON_EXPIRY_DURATION=24h")
 
     def test_leader_elected(self):
         leader_file = os.path.join(self.harness.charm.charm_dir, "juju-jimm-leader.env")

diff --git a/cmd/jimmsrv/main.go b/cmd/jimmsrv/main.go
@@ -43,19 +43,29 @@ func start(ctx context.Context, s *service.Service) error {
 		}
 	}
 
+	macaroonExpiryDuration := 24 * time.Hour
+	durationString := os.Getenv("JIMM_MACAROON_EXPIRY_DURATION")
+	if durationString != "" {
+		expiry, err := time.ParseDuration(durationString)
+		if err != nil {
+			zapctx.Error(ctx, "failed to parse macaroon expiry duration", zap.Error(err))
+		}
+		macaroonExpiryDuration = expiry
+	}
 	jimmsvc, err := jimm.NewService(ctx, jimm.Params{
-		ControllerUUID:    os.Getenv("JIMM_UUID"),
-		DSN:               os.Getenv("JIMM_DSN"),
-		CandidURL:         os.Getenv("CANDID_URL"),
-		CandidPublicKey:   os.Getenv("CANDID_PUBLIC_KEY"),
-		BakeryAgentFile:   os.Getenv("BAKERY_AGENT_FILE"),
-		ControllerAdmins:  strings.Fields(os.Getenv("JIMM_ADMINS")),
-		VaultSecretFile:   os.Getenv("VAULT_SECRET_FILE"),
-		VaultAddress:      os.Getenv("VAULT_ADDR"),
-		VaultAuthPath:     os.Getenv("VAULT_AUTH_PATH"),
-		VaultPath:         os.Getenv("VAULT_PATH"),
-		DashboardLocation: os.Getenv("JIMM_DASHBOARD_LOCATION"),
-		PublicDNSName:     os.Getenv("JIMM_DNS_NAME"),
+		ControllerUUID:         os.Getenv("JIMM_UUID"),
+		DSN:                    os.Getenv("JIMM_DSN"),
+		CandidURL:              os.Getenv("CANDID_URL"),
+		CandidPublicKey:        os.Getenv("CANDID_PUBLIC_KEY"),
+		BakeryAgentFile:        os.Getenv("BAKERY_AGENT_FILE"),
+		ControllerAdmins:       strings.Fields(os.Getenv("JIMM_ADMINS")),
+		VaultSecretFile:        os.Getenv("VAULT_SECRET_FILE"),
+		VaultAddress:           os.Getenv("VAULT_ADDR"),
+		VaultAuthPath:          os.Getenv("VAULT_AUTH_PATH"),
+		VaultPath:              os.Getenv("VAULT_PATH"),
+		DashboardLocation:      os.Getenv("JIMM_DASHBOARD_LOCATION"),
+		PublicDNSName:          os.Getenv("JIMM_DNS_NAME"),
+		MacaroonExpiryDuration: macaroonExpiryDuration,
 	})
 	if err != nil {
 		return err

diff --git a/service.go b/service.go
@@ -129,6 +129,9 @@ type Params struct {
 	// PublicDNSName is the name to advertise as the public address of
 	// the juju controller.
 	PublicDNSName string
+
+	// MacaroonExpiryDuration holds the expiry duration of authentication macaroons.
+	MacaroonExpiryDuration time.Duration
 }
 
 // A Service is the implementation of a JIMM server.
@@ -319,11 +322,19 @@ func newAuthenticator(ctx context.Context, db *db.Database, p Params) (jimm.Auth
 	if err != nil {
 		return nil, err
 	}
+
+	if p.MacaroonExpiryDuration == 0 {
+		p.MacaroonExpiryDuration = 24 * time.Hour
+	}
+
 	return auth.JujuAuthenticator{
 		Bakery: identchecker.NewBakery(identchecker.BakeryParams{
-			RootKeyStore: dbrootkeystore.NewRootKeys(100, nil).NewStore(db, dbrootkeystore.Policy{
-				ExpiryDuration: 24 * time.Hour,
-			}),
+			RootKeyStore: dbrootkeystore.NewRootKeys(100, nil).NewStore(
+				db,
+				dbrootkeystore.Policy{
+					ExpiryDuration: p.MacaroonExpiryDuration,
+				},
+			),
 			Locator:        httpbakery.NewThirdPartyLocator(nil, tps),
 			Key:            key,
 			IdentityClient: candidClient,