Merge branch 'main' into secret-owner-behaviour-change-1062

ops/testing.py

@@ -2477,9 +2477,17 @@ def _relation_id_to(self, remote_app: str) -> Optional[int]:
         return None
 
     def _ensure_secret_owner(self, secret: _Secret):
-        if secret.owner_name not in [self.app_name, self.unit_name]:
-            raise model.SecretNotFoundError(
-                f'You must own secret {secret.id!r} to perform this operation')
+        # For unit secrets, the owner unit has manage permissions. For app
+        # secrets, the leader has manage permissions and other units only have
+        # view permissions.
+        # https://discourse.charmhub.io/t/secret-access-permissions/12627
+        unit_secret = secret.owner_name == self.unit_name
+        app_secret = secret.owner_name == self.app_name
+
+        if unit_secret or (app_secret and self.is_leader()):
+            return
+        raise model.SecretNotFoundError(
+            f'You must own secret {secret.id!r} to perform this operation')
 
     def secret_info_get(self, *,
                         id: Optional[str] = None,

test/test_testing.py

@@ -4870,6 +4870,7 @@ def test_get_secret_grants(self):
         harness.add_relation_unit(relation_id, 'webapp/0')
         assert harness is not None
 
+        harness.set_leader(True)
         secret = harness.model.app.add_secret({'foo': 'x'})
         assert secret.id is not None
         self.assertEqual(harness.get_secret_grants(secret.id, relation_id), set())
@@ -4965,6 +4966,52 @@ def test_trigger_secret_expiration(self):
         with self.assertRaises(RuntimeError):
             harness.trigger_secret_removal('nosecret', 1)
 
+    def test_secret_permissions_unit(self):
+        harness = ops.testing.Harness(ops.CharmBase, meta='name: database')
+        self.addCleanup(harness.cleanup)
+        harness.begin()
+
+        # The charm can always manage a local unit secret.
+        secret_id = harness.charm.unit.add_secret({"password": "1234"}).id
+        secret = harness.charm.model.get_secret(id=secret_id)
+        self.assertEqual(secret.get_content(), {"password": "1234"})
+        info = secret.get_info()
+        self.assertEqual(info.id, secret_id)
+        secret.set_content({"password": "5678"})
+        secret.remove_all_revisions()
+
+    def test_secret_permissions_leader(self):
+        harness = ops.testing.Harness(ops.CharmBase, meta='name: database')
+        self.addCleanup(harness.cleanup)
+        harness.begin()
+
+        # The leader can manage an application secret.
+        harness.set_leader(True)
+        secret_id = harness.charm.app.add_secret({"password": "1234"}).id
+        secret = harness.charm.model.get_secret(id=secret_id)
+        self.assertEqual(secret.get_content(), {"password": "1234"})
+        info = secret.get_info()
+        self.assertEqual(info.id, secret_id)
+        secret.set_content({"password": "5678"})
+        secret.remove_all_revisions()
+
+    def test_secret_permissions_nonleader(self):
+        harness = ops.testing.Harness(ops.CharmBase, meta='name: database')
+        self.addCleanup(harness.cleanup)
+        harness.begin()
+
+        # Non-leaders can only view an application secret.
+        harness.set_leader(False)
+        secret_id = harness.charm.app.add_secret({"password": "1234"}).id
+        secret = harness.charm.model.get_secret(id=secret_id)
+        self.assertEqual(secret.get_content(), {"password": "1234"})
+        with self.assertRaises(ops.model.SecretNotFoundError):
+            secret.get_info()
+        with self.assertRaises(ops.model.SecretNotFoundError):
+            secret.set_content({"password": "5678"})
+        with self.assertRaises(ops.model.SecretNotFoundError):
+            secret.remove_all_revisions()
+
 
 class EventRecorder(ops.CharmBase):
     def __init__(self, framework: ops.Framework):