Bugfix/app registration (#1250)

* Update auth settings, and add regression test formating the github key name

* Refactor format_github_key_name to simplify logic

* Regression test for get_github_key_display_name

* Simplify logic for get_github_key_display_name

* Remove DATA_ACCOUNT_ID from created github secrets

This secret is no longer needed when registering an app, as the
base images are no longer stored in ECR. Instead they are made
public in Docker Hub

* Auto-prefix new app datasource names with the env

* Hide manage customers button until auth0 client exists

* Redirect to manage app page after registering an app

controlpanel/api/cluster.py

@@ -389,7 +389,6 @@ class App(EntityResource):
     AUTHENTICATION_REQUIRED = "AUTHENTICATION_REQUIRED"
     AUTH0_PASSWORDLESS = "AUTH0_PASSWORDLESS"
     APP_ROLE_ARN = "APP_ROLE_ARN"
-    DATA_ACCOUNT_ID = 'DATA_ACCOUNT_ID'
 
     def __init__(self, app, github_api_token=None, auth0_instance=None):
         super(App, self).__init__()
@@ -415,7 +414,6 @@ def _create_secrets(self, env_name, client=None):
         secret_data: dict = {
             App.IP_RANGES: self.app.env_allowed_ip_ranges(env_name=env_name),
             App.APP_ROLE_ARN: self.app.iam_role_arn,
-            App.DATA_ACCOUNT_ID: settings.AWS_DATA_ACCOUNT_ID
         }
         if client:
             secret_data[App.AUTH0_CLIENT_ID] = client["client_id"]
@@ -534,24 +532,27 @@ def format_github_key_name(key_name):
         Format the self-defined secret/variable by adding prefix if
         create/update value back to github and there is no prefix in the name
         """
-        if key_name not in settings.AUTH_SETTINGS_ENVS \
-                and key_name not in settings.AUTH_SETTINGS_SECRETS:
-            if not key_name.startswith(settings.APP_SELF_DEFINE_SETTING_PREFIX):
-                return f"{settings.APP_SELF_DEFINE_SETTING_PREFIX}{key_name}"
-        return key_name
+        if key_name in settings.AUTH_SETTINGS_ENVS:
+            return key_name
+
+        if key_name in settings.AUTH_SETTINGS_SECRETS:
+            return key_name
+
+        if key_name.startswith(settings.APP_SELF_DEFINE_SETTING_PREFIX):
+            return key_name
+
+        return f"{settings.APP_SELF_DEFINE_SETTING_PREFIX}{key_name}"
 
     @staticmethod
-    def get_github_key_display_name(key_name):
+    def get_github_key_display_name(key_name: str) -> str:
         """
         Format the self-defined secret/variable by removing the prefix
         if reading it from github and there is prefix in the name
         """
-        if key_name and key_name not in settings.AUTH_SETTINGS_ENVS \
-                and key_name not in settings.AUTH_SETTINGS_SECRETS:
-            if settings.APP_SELF_DEFINE_SETTING_PREFIX in key_name:
-                return key_name.replace(
-                    settings.APP_SELF_DEFINE_SETTING_PREFIX, "")
-        return key_name
+        if not key_name.startswith(settings.APP_SELF_DEFINE_SETTING_PREFIX):
+            return key_name
+
+        return key_name.replace(settings.APP_SELF_DEFINE_SETTING_PREFIX, "", 1)
 
     def create_or_update_secret(self, env_name, secret_key, secret_value):
         org_name, repo_name = extract_repo_info_from_url(self.app.repo_url)

controlpanel/api/models/app.py

@@ -83,6 +83,12 @@ def release_name(self):
     def iam_role_arn(self):
         return cluster.iam_arn(f"role/{self.iam_role_name}")
 
+    @property
+    def can_manage_customers(self):
+        if not self.app_conf:
+            return False
+        return bool(self.app_conf.get(self.KEY_WORD_FOR_AUTH_SETTINGS))
+
     def get_group_id(self, env_name):
         return self.get_auth_client(env_name).get("group_id")
 

controlpanel/frontend/jinja2/includes/app-list.html

@@ -36,10 +36,12 @@
       <td class="govuk-table__cell">
         {{ yes_no(user.is_app_admin(app.id)) }}
       </td>
-      <td class="govuk-table__cell">
-        <a href="{{ url("appcustomers-page", kwargs={"pk": app.id, "page_no": "1"}) }}"
-          class="govuk-button govuk-button--secondary right" >Manage customers</a>
-      </td>
+      {% if app.can_manage_customers %}
+        <td class="govuk-table__cell">
+          <a href="{{ url("appcustomers-page", kwargs={"pk": app.id, "page_no": "1"}) }}"
+            class="govuk-button govuk-button--secondary right" >Manage customers</a>
+        </td>
+      {% endif %}
       <td class="govuk-table__cell">
         <a class="govuk-button govuk-button--secondary right {%- if not request.user.has_perm('api.retrieve_app', app) %} govuk-visually-hidden{% endif %}"
           href="{{ url("manage-app", kwargs={ "pk": app.id}) }}">

controlpanel/frontend/jinja2/webapp-create.html

@@ -23,7 +23,12 @@
       "text": '60 chars max, only lowercase letters, numbers, periods and hyphens, auto-prefixed with "' + env + '"'
     },
     "errorMessage": {"text": form.new_datasource_name.errors|join(". ")} if form.new_datasource_name.errors else {},
-    "value": form.new_datasource_name.value()
+    "value": form.new_datasource_name.value(),
+    "attributes": {
+      "data-bucket-prefix": env + "-",
+      "pattern": "[a-z0-9.-]{1,60}",
+      "maxlength": "60",
+    }
   }) }}
   </div>
 {% endset %}

controlpanel/frontend/views/app.py

@@ -151,7 +151,7 @@ def get_success_url(self):
             self.request,
             f"Successfully registered {self.object.name} webapp",
         )
-        return reverse_lazy("list-apps")
+        return reverse_lazy("manage-app", kwargs={"pk": self.object.pk})
 
     def form_valid(self, form):
         try:

controlpanel/frontend/views/app_variables.py

@@ -37,7 +37,7 @@ def get_form_kwargs(self):
         kwargs["initial"]["env_name"] = data.get("env_name")
         kwargs["initial"]["key"] = self.kwargs.get("var_name")
         kwargs["initial"]["display_key"] = cluster.App.get_github_key_display_name(
-            self.kwargs.get("var_name"))
+            self.kwargs.get("var_name", ""))
         if kwargs["initial"]["key"]:
             try:
                 var_info = cluster.App(

controlpanel/frontend/views/secrets.py

@@ -40,7 +40,7 @@ def get_form_kwargs(self):
         kwargs["initial"]["env_name"] = data.get("env_name")
         kwargs["initial"]["key"] = self.kwargs.get("secret_name")
         kwargs["initial"]["display_key"] = cluster.App.get_github_key_display_name(
-            self.kwargs.get("secret_name"))
+            self.kwargs.get("secret_name", ""))
         return kwargs
 
     def get_success_url(self, app_id):

settings.yaml

@@ -95,12 +95,14 @@ AUTH_SETTINGS_SECRETS:
   - AUTH0_CLIENT_ID
   - AUTH0_CLIENT_SECRET
   - IP_RANGES
+  - APP_ROLE_ARN
 
 AUTH_SETTINGS_NO_EDIT:
   - AUTH0_CLIENT_ID
   - AUTH0_CLIENT_SECRET
   - AUTH0_DOMAIN
   - AUTH0_PASSWORDLESS
+  - APP_ROLE_ARN
 
 AUTH_SETTINGS_ENVS:
   - AUTH0_DOMAIN

tests/api/cluster/test_app.py

@@ -163,7 +163,6 @@ def test_create_secrets(app):
     secrets = {
         app_cluster.IP_RANGES: "1.2.3",
         app_cluster.APP_ROLE_ARN: app.iam_role_arn,
-        app_cluster.DATA_ACCOUNT_ID: settings.AWS_DATA_ACCOUNT_ID
     }
     with patch.object(app_cluster, "create_or_update_secrets"):
         app_cluster._create_secrets(env_name="dev", client=None)
@@ -173,6 +172,31 @@ def test_create_secrets(app):
         )
 
 
+@pytest.mark.parametrize("key, expected", [
+    ("AUTH0_CLIENT_ID", "AUTH0_CLIENT_ID"),
+    ("AUTH0_CLIENT_SECRET", "AUTH0_CLIENT_SECRET"),
+    ("AUTH0_DOMAIN", "AUTH0_DOMAIN"),
+    ("AUTH0_PASSWORDLESS", "AUTH0_PASSWORDLESS"),
+    ("APP_ROLE_ARN", "APP_ROLE_ARN"),
+    ("CUSTOM_SETTING", "XXX_CUSTOM_SETTING"),
+])
+def test_format_github_key_name(key, expected):
+    assert cluster.App(None).format_github_key_name(key_name=key) == expected
+
+
+@pytest.mark.parametrize("key, expected", [
+    ("AUTH0_CLIENT_ID", "AUTH0_CLIENT_ID"),
+    ("AUTH0_CLIENT_SECRET", "AUTH0_CLIENT_SECRET"),
+    ("AUTH0_DOMAIN", "AUTH0_DOMAIN"),
+    ("AUTH0_PASSWORDLESS", "AUTH0_PASSWORDLESS"),
+    ("APP_ROLE_ARN", "APP_ROLE_ARN"),
+    ("XXX_CUSTOM_SETTING", "CUSTOM_SETTING"),
+    ("XXX_XXX_CUSTOM_SETTING", "XXX_CUSTOM_SETTING"),
+])
+def test_get_github_key_display_name(key, expected):
+    assert cluster.App(None).get_github_key_display_name(key) == expected
+
+
 # TODO can this be removed?
 mock_ingress = MagicMock(name="Ingress")
 mock_ingress.spec.rules = [MagicMock(name="Rule", host="test-app.example.com")]

tests/api/models/test_app.py

@@ -209,3 +209,17 @@ def test_app_allowed_ip_ranges():
 def test_iam_role_arn():
     app = App(slug="example-app")
     assert app.iam_role_arn == f"arn:aws:iam::{settings.AWS_DATA_ACCOUNT_ID}:role/test_app_example-app"
+
+
+@pytest.mark.parametrize("conf, expected", [
+    (None, False),
+    ({}, False),
+    ({"some_setting_key": []}, False),
+    ({App.KEY_WORD_FOR_AUTH_SETTINGS: None}, False),
+    ({App.KEY_WORD_FOR_AUTH_SETTINGS: {}}, False),
+    ({App.KEY_WORD_FOR_AUTH_SETTINGS: {"dev": {}}}, True),
+])
+def test_can_manage_customers(conf, expected):
+    app = App()
+    app.app_conf = conf
+    assert app.can_manage_customers is expected

tests/frontend/views/test_app.py

@@ -721,6 +721,9 @@ def test_register_app_with_creating_datasource(client, users):
     related_bucket_ids = [a.s3bucket_id for a in created_app.apps3buckets.all()]
     assert len(related_bucket_ids) == 1
     assert bucket.id in related_bucket_ids
+    assert response.url == reverse(
+        "manage-app", kwargs={"pk": created_app.pk}
+    )
 
 
 def test_register_app_invalid_organisation(client, users):
@@ -731,7 +734,8 @@ def test_register_app_invalid_organisation(client, users):
         connect_bucket="later",
     )
 
-    response = client.post(reverse("create-app"), data)
+    url = reverse("create-app")
+    response = client.post(url, data)
 
     # 200 due to errors
     assert response.status_code == 200