From 2879537c30ba59ee1e044887f0f5af54e3f6cca5 Mon Sep 17 00:00:00 2001
From: Joey Orlando <joey.orlando@grafana.com>
Date: Thu, 4 May 2023 12:38:26 -0400
Subject: [PATCH] properly parse grafana cloud feature toggles (#1880)

# What this PR does

## Which issue(s) this PR fixes

## Checklist

- [x] Unit, integration, and e2e (if applicable) tests updated
- [ ] Documentation added (or `pr:no public docs` PR label added if not
required) (N/A)
- [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not
required)
---
 CHANGELOG.md                                  |  4 +++
 engine/apps/grafana_plugin/helpers/client.py  | 19 +++++++++-
 .../tests/test_gcom_api_client.py             | 35 +++++++++++++++++++
 3 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8afe958d32..7ca4d3c9fd 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Improve ical comparison when checking for imported ical updates ([1870](https://github.com/grafana/oncall/pull/1870))
 
+### Fixed
+
+- Fix issue with how OnCall determines if a cloud Grafana Instance supports RBAC by @joeyorlando ([#1880](https://github.com/grafana/oncall/pull/1880))
+
 ## v1.2.18 (2023-05-03)
 
 ### Added
diff --git a/engine/apps/grafana_plugin/helpers/client.py b/engine/apps/grafana_plugin/helpers/client.py
index 4e86830bac..592ccaeda6 100644
--- a/engine/apps/grafana_plugin/helpers/client.py
+++ b/engine/apps/grafana_plugin/helpers/client.py
@@ -217,6 +217,23 @@ def get_instance_info(self, stack_id: str, include_config_query_param: bool = Fa
         data, _ = self.api_get(url)
         return data
 
+    def _feature_toggle_is_enabled(self, instance_info: GCOMInstanceInfo, feature_name: str) -> bool:
+        """
+        there are two ways that feature toggles can be enabled, this method takes into account both
+        https://grafana.com/docs/grafana/latest/setup-grafana/configure-grafana/#enable
+        """
+        instance_feature_toggles = instance_info.get("config", {}).get("feature_toggles", {})
+
+        if not instance_feature_toggles:
+            return False
+
+        # features enabled via enable key are space separated
+        features_enabled_via_enable_key = instance_feature_toggles.get("enable", "").split(" ")
+        feature_enabled_via_enable_key = feature_name in features_enabled_via_enable_key
+        feature_enabled_via_direct_key = instance_feature_toggles.get(feature_name, "false") == "true"
+
+        return feature_enabled_via_enable_key or feature_enabled_via_direct_key
+
     def is_rbac_enabled_for_stack(self, stack_id: str) -> bool:
         """
         NOTE: must use an "Admin" GCOM token when calling this method
@@ -224,7 +241,7 @@ def is_rbac_enabled_for_stack(self, stack_id: str) -> bool:
         instance_info = self.get_instance_info(stack_id, True)
         if not instance_info:
             return False
-        return instance_info.get("config", {}).get("feature_toggles", {}).get("accessControlOnCall", "false") == "true"
+        return self._feature_toggle_is_enabled(instance_info, "accessControlOnCall")
 
     def get_instances(self, query: str):
         return self.api_get(query)
diff --git a/engine/apps/grafana_plugin/tests/test_gcom_api_client.py b/engine/apps/grafana_plugin/tests/test_gcom_api_client.py
index cac834d88b..0df6ae31c0 100644
--- a/engine/apps/grafana_plugin/tests/test_gcom_api_client.py
+++ b/engine/apps/grafana_plugin/tests/test_gcom_api_client.py
@@ -6,6 +6,8 @@
 
 
 class TestIsRbacEnabledForStack:
+    TEST_FEATURE_TOGGLE = "helloWorld"
+
     @pytest.mark.parametrize(
         "gcom_api_response,expected",
         [
@@ -27,3 +29,36 @@ def test_it_returns_based_on_feature_toggle_value(
         api_client = GcomAPIClient("someFakeApiToken")
         assert api_client.is_rbac_enabled_for_stack(stack_id) == expected
         assert mocked_gcom_api_client_api_get.called_once_with(f"instances/{stack_id}?config=true")
+
+    @pytest.mark.parametrize(
+        "instance_info,expected",
+        [
+            ({}, False),
+            ({"config": {}}, False),
+            ({"config": {"feature_toggles": {}}}, False),
+            ({"config": {"feature_toggles": {"enable": "foo bar baz"}}}, False),
+            ({"config": {"feature_toggles": {TEST_FEATURE_TOGGLE: "false"}}}, False),
+            # must be space separated
+            ({"config": {"feature_toggles": {"enable": f"foo bar {TEST_FEATURE_TOGGLE}baz"}}}, False),
+            # these cases will probably never happen, but lets account for them anyways
+            (
+                {
+                    "config": {
+                        "feature_toggles": {
+                            "enable": f"foo bar baz {TEST_FEATURE_TOGGLE}",
+                            TEST_FEATURE_TOGGLE: "false",
+                        }
+                    }
+                },
+                True,
+            ),
+            ({"config": {"feature_toggles": {"enable": f"foo bar baz", TEST_FEATURE_TOGGLE: "true"}}}, True),
+            ({"config": {"feature_toggles": {"enable": f"foo bar {TEST_FEATURE_TOGGLE} baz"}}}, True),
+            ({"config": {"feature_toggles": {TEST_FEATURE_TOGGLE: "true"}}}, True),
+        ],
+    )
+    def test_feature_toggle_is_enabled(self, instance_info, expected) -> None:
+        assert (
+            GcomAPIClient("someFakeApiToken")._feature_toggle_is_enabled(instance_info, self.TEST_FEATURE_TOGGLE)
+            == expected
+        )