Add an admin API endpoint to support per-user feature flags

docs/admin_api/experimental_features.md

@@ -13,33 +13,42 @@ for a server admin: see [Admin API](../usage/administration/admin_api/).
 
 ## Enabling/Disabling Features
 
-This API allows a server administrator to enable experimental features for a given user, where the
-`user_id` is the user id of the user for whom to enable or disable the features. The request must 
-provide a body listing the features to enable/disable in the following format:
+This API allows a server administrator to enable experimental features for a given user. The request must 
+provide a body containing the user id and listing the features to enable/disable in the following format:
 ```
 {
-"features": {"msc3026": True, "msc2654": True}
+"user_id": user_id,
+"features": {"msc3026": true, "msc2654": true}
 }
 
 ```
-where True is  used to enable the feature, and False is used to disable the feature.
+where true is  used to enable the feature, and false is used to disable the feature.
 
 
 The API is:
 
 ```
-PUT /_synapse/admin/v1/experimental_features/<user_id>
+PUT /_synapse/admin/v1/experimental_features
 ```
 
 ## Listing Enabled Features
 
-To list the enabled features for a given user, use the following API:
+To list which features are enabled/disabled for a given user, send a request with a body in the following format:
+
+```
+{
+"user_id": user_id
+}
+```
+
+to the API
 
 ```
 GET /_synapse/admin/v1/experimental_features
 ```
 
-It will return a list of enabled features in the following format:
+It will return a list of possible features and indicate whether they are enabled or disabled for the
+user like so:
 ```
-{"user_id": user_id, "features": ["msc3026"]}
+{"user_id": user_id, "features": {"msc3026": true, "msc2654": true, "msc3881": false, "msc3967": false}}
 ```

synapse/rest/admin/experimental_features.py

@@ -44,7 +44,7 @@ class ExperimentalFeaturesRestServlet(RestServlet):
     for a given user
     """
 
-    PATTERNS = admin_patterns("/experimental_features/(?P<user_id>[^/]*)")
+    PATTERNS = admin_patterns("/experimental_features")
 
     def __init__(self, hs: "HomeServer"):
         super().__init__()
@@ -55,40 +55,62 @@ def __init__(self, hs: "HomeServer"):
     async def on_GET(
         self,
         request: SynapseRequest,
-        user_id: str,
     ) -> Tuple[int, JsonDict]:
         """
         List which features are enabled for a given user
         """
         await assert_requester_is_admin(self.auth, request)
 
+        body = parse_json_object_from_request(request)
+
+        if "user_id" not in body:
+            raise SynapseError(
+                HTTPStatus.BAD_REQUEST,
+                "Missing property 'user_id' in the request body",
+            )
+
+        user_id = body["user_id"]
+
         target_user = UserID.from_string(user_id)
         if not self.is_mine(target_user):
             raise SynapseError(
                 HTTPStatus.BAD_REQUEST,
                 "User must be local to check what experimental features are enabled.",
             )
 
-        enabled_list = await self.store.list_enabled_features(user_id)
+        enabled_features = await self.store.list_enabled_features(user_id)
 
-        return HTTPStatus.OK, {"user_id": user_id, "features": enabled_list}
+        user_features = {}
+        for feature in ExperimentalFeature:
+            if feature in enabled_features:
+                user_features[feature] = True
+            else:
+                user_features[feature] = False
+        return HTTPStatus.OK, {"user_id": user_id, "features": user_features}
 
-    async def on_PUT(
-        self, request: SynapseRequest, user_id: str
-    ) -> Tuple[HTTPStatus, Dict]:
+    async def on_PUT(self, request: SynapseRequest) -> Tuple[HTTPStatus, Dict]:
         """
         Enable or disable the provided features for the requester
         """
         await assert_requester_is_admin(self.auth, request)
 
+        body = parse_json_object_from_request(request)
+
+        if "user_id" not in body:
+            raise SynapseError(
+                HTTPStatus.BAD_REQUEST,
+                "Missing property 'user_id' in the request body",
+            )
+
+        user_id = body["user_id"]
+
         target_user = UserID.from_string(user_id)
         if not self.is_mine(target_user):
             raise SynapseError(
                 HTTPStatus.BAD_REQUEST,
                 "User must be local to enable experimental features.",
             )
 
-        body = parse_json_object_from_request(request)
         features = body.get("features")
         if not features:
             raise SynapseError(

tests/rest/admin/test_admin.py

@@ -394,11 +394,14 @@ def test_enable_and_disable(self) -> None:
         Test basic functionality of ExperimentalFeatures endpoint
         """
         # test enabling features works
-        url = f"{self.url}/{self.other_user}"
+        url = f"{self.url}"
         channel = self.make_request(
             "PUT",
             url,
-            content={"features": {"msc3026": True, "msc2654": True}},
+            content={
+                "user_id": self.other_user,
+                "features": {"msc3026": True, "msc2654": True},
+            },
             access_token=self.admin_user_tok,
         )
         self.assertEqual(channel.code, 200)
@@ -409,70 +412,80 @@ def test_enable_and_disable(self) -> None:
         channel = self.make_request(
             "GET",
             url,
+            content={"user_id": self.other_user},
             access_token=self.admin_user_tok,
         )
         self.assertEqual(channel.code, 200)
-        self.assertIn(
-            "msc3026",
-            channel.json_body["features"],
+        self.assertEqual(
+            True,
+            channel.json_body["features"]["msc3026"],
         )
-        self.assertIn(
-            "msc2654",
-            channel.json_body["features"],
+        self.assertEqual(
+            True,
+            channel.json_body["features"]["msc2654"],
         )
         self.assertIn(
             self.other_user,
             channel.json_body["user_id"],
         )
 
         # test disabling a feature works
-        url = f"{self.url}/{self.other_user}"
+        url = f"{self.url}"
         channel = self.make_request(
             "PUT",
             url,
-            content={"features": {"msc3026": False}},
+            content={"user_id": self.other_user, "features": {"msc3026": False}},
             access_token=self.admin_user_tok,
         )
         self.assertEqual(channel.code, 200)
 
-        # list which features are enabled and ensure the disabled one is not listed
+        # list the features enabled/disabled and ensure they are still are correct
         self.assertEqual(channel.code, 200)
-        url = f"{self.url}/{self.other_user}"
+        url = f"{self.url}"
         channel = self.make_request(
             "GET",
             url,
+            content={"user_id": self.other_user},
             access_token=self.admin_user_tok,
         )
         self.assertEqual(channel.code, 200)
-        self.assertNotIn(
-            "msc3026",
-            channel.json_body["features"],
+        self.assertEqual(
+            False,
+            channel.json_body["features"]["msc3026"],
         )
-        self.assertIn(
-            "msc2654",
-            channel.json_body["features"],
+        self.assertEqual(
+            True,
+            channel.json_body["features"]["msc2654"],
+        )
+        self.assertEqual(
+            False,
+            channel.json_body["features"]["msc3881"],
+        )
+        self.assertEqual(
+            False,
+            channel.json_body["features"]["msc3967"],
         )
         self.assertIn(
             self.other_user,
             channel.json_body["user_id"],
         )
 
         # test nothing blows up if you try to disable a feature that isn't already enabled
-        url = f"{self.url}/{self.other_user}"
+        url = f"{self.url}"
         channel = self.make_request(
             "PUT",
             url,
-            content={"features": {"msc3026": False}},
+            content={"user_id": self.other_user, "features": {"msc3026": False}},
             access_token=self.admin_user_tok,
         )
         self.assertEqual(channel.code, 200)
 
         # test trying to enable a feature without an admin access token is denied
-        url = f"{self.url}/{self.other_user}"
+        url = f"{self.url}"
         channel = self.make_request(
             "PUT",
             url,
-            content={"features": {"msc3881": True}},
+            content={"user_id": self.other_user, "features": {"msc3881": True}},
             access_token=self.other_user_tok,
         )
         self.assertEqual(channel.code, 403)
@@ -482,11 +495,11 @@ def test_enable_and_disable(self) -> None:
         )
 
         # test trying to enable a bogus msc is denied
-        url = f"{self.url}/{self.other_user}"
+        url = f"{self.url}"
         channel = self.make_request(
             "PUT",
             url,
-            content={"features": {"msc6666": True}},
+            content={"user_id": self.other_user, "features": {"msc6666": True}},
             access_token=self.admin_user_tok,
         )
         self.assertEqual(channel.code, 400)