ensure secret names are unique and rfc1123 compliant (#138)

* add rfc1123 helper and ensure secret id is compliant

* unit test for generate_rfc1123

* tweak secret_id format

* validate name during user_create

* ensure generate_rfc1123 starts/ends with alphanum

* adjust secret name to start with "auth-" to ensure usernames with non-rfc1123 chars do not start the name; drop unnecessary suffix since the generator ensures a valid end char

actions.yaml

@@ -63,7 +63,9 @@ user-create:
   params:
     name:
       type: string
-      description: Username for the new user
+      description: |
+        Username for the new user. This value must only contain alphanumeric
+        characters, '@', '-' or '.'.
       minLength: 2
     groups:
       type: string

actions/user_actions.py

@@ -1,6 +1,7 @@
 #!/usr/local/sbin/charm-env python3
 import json
 import os
+import re
 import sys
 from base64 import b64decode
 from charmhelpers.core import hookenv
@@ -62,11 +63,19 @@ def user_create():
         action_fail('User "{}" already exists.'.format(user))
         return
 
+    # Validate the name
+    if re.search('[^0-9A-Za-z@.-]+', user):
+        msg = "User name may only contain alphanumeric characters, '@', '-' or '.'"
+        action_fail(msg)
+        return
+
     # Create the secret
     # TODO: make the token format less magical so it doesn't get out of
     # sync with the function that creates secrets in k8s-master.py.
     token = '{}::{}'.format(user, layer.kubernetes_master.token_generator())
-    layer.kubernetes_master.create_secret(token, user, user, groups)
+    if not layer.kubernetes_master.create_secret(token, user, user, groups):
+        action_fail('Failed to create secret for: {}'.format(user))
+        return
 
     # Create a kubeconfig
     ca_crt = layer.kubernetes_common.ca_crt_path

lib/charms/layer/kubernetes_master.py

@@ -22,7 +22,6 @@
 AUTH_BACKUP_EXT = 'pre-secrets'
 AUTH_BASIC_FILE = '/root/cdk/basic_auth.csv'
 AUTH_SECRET_NS = 'kube-system'
-AUTH_SECRET_SUFFIX = 'token-auth'
 AUTH_SECRET_TYPE = 'juju.is/token-auth'
 AUTH_TOKENS_FILE = '/root/cdk/known_tokens.csv'
 STANDARD_API_PORT = 6443
@@ -190,6 +189,27 @@ def migrate_auth_file(filename):
     return True
 
 
+def generate_rfc1123(length=10):
+    '''Generate a random string compliant with RFC 1123.
+
+    https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-subdomain-names
+
+    param: length - the length of the string to generate
+    '''
+    length = 253 if length > 253 else length
+    first_last_opts = string.ascii_lowercase + string.digits
+    middle_opts = first_last_opts + '-' + '.'
+
+    # ensure first and last chars are alphanum
+    length -= 2
+    rand_str = (
+        random.SystemRandom().choice(first_last_opts) +
+        ''.join(random.SystemRandom().choice(middle_opts) for _ in range(length)) +
+        random.SystemRandom().choice(first_last_opts)
+    )
+    return rand_str
+
+
 def token_generator(length=32):
     '''Generate a random token for use in account tokens.
 
@@ -234,9 +254,9 @@ def create_known_token(token, username, user, groups=None):
 
 
 def create_secret(token, username, user, groups=None):
-    # secret names can only include alphanum and hyphens
-    sani_name = re.sub('[^0-9a-zA-Z]+', '-', user)
-    secret_id = '{}-{}'.format(sani_name, AUTH_SECRET_SUFFIX)
+    # secret IDs must be unique and rfc1123 compliant
+    sani_name = re.sub('[^0-9a-z.-]+', '-', user.lower())
+    secret_id = 'auth-{}-{}'.format(sani_name, generate_rfc1123(10))
     # The authenticator expects tokens to be in the form user::token
     token_delim = '::'
     if token_delim not in token:
@@ -257,8 +277,10 @@ def create_secret(token, username, user, groups=None):
 
         if kubernetes_common.kubectl_manifest('apply', tmp_manifest.name):
             hookenv.log("Created secret for {}".format(username))
+            return True
         else:
             hookenv.log("WARN: Unable to create secret for {}".format(username))
+            return False
 
 
 def delete_secret(secret_id):

tests/test_kubernetes_master_actions.py

@@ -54,6 +54,13 @@ def test_user_create(mock_get, mock_master, mock_common, mock_chmod):
         user_actions.user_create()
         assert user_actions.action_fail.called
 
+    # Ensure failure when user name is invalid
+    mock_get.return_value = 'FunnyBu;sness'
+    with mock.patch('actions.user_actions.user_list',
+                    return_value=test_data):
+        user_actions.user_create()
+        assert user_actions.action_fail.called
+
     # Ensure calls/args when we have a new user
     user = 'newuser'
     password = 'password'

tests/test_kubernetes_master_lib.py

@@ -1,6 +1,7 @@
 import base64
 import json
 import pytest
+import re
 import tempfile
 from pathlib import Path
 from unittest import mock
@@ -74,6 +75,12 @@ def test_delete_secret(mock_kubectl):
     assert secret_ns in args
 
 
+def test_generate_rfc1123():
+    """Verify genereated string is RFC 1123 compliant."""
+    id = charmlib.generate_rfc1123()
+    assert re.search('[^0-9a-z.-]+', id) is None
+
+
 def test_get_csv_password(auth_file):
     """Verify expected content from an auth file is returned."""
     password = 'password'