From ba68911df5a2f5a3e00f5c3579cf2aed4f760959 Mon Sep 17 00:00:00 2001 From: Teddy Andrieux Date: Tue, 19 Nov 2019 16:41:58 +0100 Subject: [PATCH 1/3] salt/auth: Add groups support for bearer auth Currently we have no permissions when using bearer token auth, add a groups function to authenticate bearer auth using the same approach as the basic auth for the moment --- salt/_auth/kubernetes_rbac.py | 76 +++++++++++++++++++++++++---------- 1 file changed, 55 insertions(+), 21 deletions(-) diff --git a/salt/_auth/kubernetes_rbac.py b/salt/_auth/kubernetes_rbac.py index 705aa6b0f1..c70ca166d8 100644 --- a/salt/_auth/kubernetes_rbac.py +++ b/salt/_auth/kubernetes_rbac.py @@ -58,6 +58,40 @@ def _check_k8s_creds(kubeconfig, token): raise +def _check_node_admin(kubeconfig): + client = kubernetes.client.ApiClient(configuration=kubeconfig) + + authz_api = kubernetes.client.AuthorizationV1Api(api_client=client) + + result = authz_api.create_self_subject_access_review( + body=kubernetes.client.V1SelfSubjectAccessReview( + spec=kubernetes.client.V1SelfSubjectAccessReviewSpec( + resource_attributes=kubernetes.client.V1ResourceAttributes( + resource='nodes', + verb='*', + ), + ), + ), + ) + + return result.status.allowed + + +AVAILABLES_GROUPS = { + 'node-admins': _check_node_admin +} + + +def _get_groups(kubeconfig): + groups = set() + + for group, func in AVAILABLES_GROUPS.items(): + if func(kubeconfig): + groups.add(group) + + return list(groups) + + @_log_exceptions def _auth_basic(kubeconfig, username, token): decoded = base64.decodestring(token) @@ -86,27 +120,7 @@ def _groups_basic(kubeconfig, username, token): kubeconfig.cert_file = None kubeconfig.key_file = None - client = kubernetes.client.ApiClient(configuration=kubeconfig) - - authz_api = kubernetes.client.AuthorizationV1Api(api_client=client) - - groups = set() - - result = authz_api.create_self_subject_access_review( - body=kubernetes.client.V1SelfSubjectAccessReview( - spec=kubernetes.client.V1SelfSubjectAccessReviewSpec( - resource_attributes=kubernetes.client.V1ResourceAttributes( - resource='nodes', - verb='*', - ), - ), - ), - ) - - if result.status.allowed: - groups.add('node-admins') - - return list(groups) + return _get_groups(kubeconfig) AUTH_HANDLERS['basic'] = { @@ -114,14 +128,34 @@ def _groups_basic(kubeconfig, username, token): 'groups': _groups_basic, } + @_log_exceptions def _auth_bearer(kubeconfig, username, token): return _check_k8s_creds(kubeconfig, 'Bearer {}'.format(token)) + +@_log_exceptions +def _groups_bearer(kubeconfig, _username, token): + kubeconfig.api_key = { + 'authorization': token, + } + kubeconfig.api_key_prefix = { + 'authorization': 'Bearer', + } + kubeconfig.username = None + kubeconfig.password = None + kubeconfig.cert_file = None + kubeconfig.key_file = None + + return _get_groups(kubeconfig) + + AUTH_HANDLERS['bearer'] = { 'auth': _auth_bearer, + 'groups': _groups_bearer } + @_log_exceptions def _load_kubeconfig(opts): config = { From c2d62ac7749001667b451ab074459a5f26575583 Mon Sep 17 00:00:00 2001 From: Teddy Andrieux Date: Thu, 28 Nov 2019 11:13:19 +0100 Subject: [PATCH 2/3] tests: Add Admin ServiceAccount fixture Add a fixture to create an Admin ServiceAccount for test purpose. This fixture create a ServiceAccount and a ClusterRoleBinding to bind this new ServiceAccount to `cluster-admin` ClusterRole --- tests/conftest.py | 105 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) diff --git a/tests/conftest.py b/tests/conftest.py index a5903ee699..a66ebe09b2 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -102,6 +102,111 @@ def test_something(k8s_client): return api_cls(api_client=k8s_apiclient) +@pytest.fixture +def admin_sa(k8s_client, k8s_apiclient): + """Fixture to create a ServiceAccount which is bind to `cluster-admin` + ClusterRole and return the ServiceAccount name + """ + rbac_k8s_client = kubernetes.client.RbacAuthorizationV1Api( + api_client=k8s_apiclient + ) + sa_name = 'test-admin' + sa_namespace = 'default' + sa_manifest = { + 'apiVersion': 'v1', + 'kind': 'ServiceAccount', + 'metadata': { + 'name': sa_name, + 'namespace': sa_namespace + } + } + crb_manifest = { + 'apiVersion': 'rbac.authorization.k8s.io/v1', + 'kind': 'ClusterRoleBinding', + 'metadata': { + 'name': sa_name + }, + 'roleRef': { + 'apiGroup': 'rbac.authorization.k8s.io', + 'kind': 'ClusterRole', + 'name': 'cluster-admin' + }, + 'subjects': [ + { + 'kind': 'ServiceAccount', + 'name': sa_name, + 'namespace': sa_namespace + } + ] + } + + k8s_client.create_namespaced_service_account( + body=sa_manifest, + namespace=sa_namespace + ) + rbac_k8s_client.create_cluster_role_binding(body=crb_manifest) + + def _check_crb_exists(): + try: + rbac_k8s_client.read_cluster_role_binding(name=sa_name) + except kubernetes.client.rest.ApiException as err: + if err.status == 404: + raise AssertionError('ClusterRoleBinding not yet created') + raise + + def _check_sa_exists(): + try: + sa_obj = k8s_client.read_namespaced_service_account( + name=sa_name, + namespace=sa_namespace + ) + except kubernetes.client.rest.ApiException as err: + if err.status == 404: + raise AssertionError('ServiceAccount not yet created') + raise + + assert sa_obj.secrets + assert sa_obj.secrets[0].name + + try: + secret_obj = k8s_client.read_namespaced_secret( + sa_obj.secrets[0].name, + sa_namespace + ) + except kubernetes.client.rest.ApiException as err: + if err.status == 404: + raise AssertionError('Secret not yet created') + raise + + assert secret_obj.data.get('token') + + # Wait for ClusterRoleBinding to exists + utils.retry(_check_crb_exists, times=20, wait=3) + + # Wait for ServiceAccount to exists + utils.retry(_check_sa_exists, times=20, wait=3) + + yield (sa_name, sa_namespace) + + try: + rbac_k8s_client.delete_cluster_role_binding( + name=sa_name, + body=kubernetes.client.V1DeleteOptions( + propagation_policy='Foreground' + ) + ) + except kubernetes.client.rest.ApiException: + pass + + k8s_client.delete_namespaced_service_account( + name=sa_name, + namespace=sa_namespace, + body=kubernetes.client.V1DeleteOptions( + propagation_policy='Foreground' + ) + ) + + @pytest.fixture(scope='module') def bootstrap_config(host): with host.sudo(): From 9d7e63d7d7f3e7601c0af02d2b1a61ae5ab73c82 Mon Sep 17 00:00:00 2001 From: Teddy Andrieux Date: Thu, 28 Nov 2019 11:29:48 +0100 Subject: [PATCH 3/3] tests: Add salt-api test for Bearer token of Admin SA Add test that a ServiceAccount bind to `cluster-admin` ClusterRole is able to everything with salt-api --- tests/post/features/salt_api.feature | 9 ++++++ tests/post/steps/test_salt_api.py | 43 +++++++++++++++++++++------- 2 files changed, 42 insertions(+), 10 deletions(-) diff --git a/tests/post/features/salt_api.feature b/tests/post/features/salt_api.feature index e5972addfc..f806e8c629 100644 --- a/tests/post/features/salt_api.feature +++ b/tests/post/features/salt_api.feature @@ -9,6 +9,15 @@ Feature: SaltAPI And we have '@runner' perms And we have '@jobs' perms + Scenario: Login to SaltAPI using an admin ServiceAccount + Given the Kubernetes API is available + When we login to SaltAPI with an admin ServiceAccount + Then we can ping all minions + And we can invoke '[".*"]' on '*' + And we have '@wheel' perms + And we have '@runner' perms + And we have '@jobs' perms + Scenario: Login to SaltAPI using a ServiceAccount Given the Kubernetes API is available When we login to SaltAPI with the ServiceAccount 'storage-operator' diff --git a/tests/post/steps/test_salt_api.py b/tests/post/steps/test_salt_api.py index ac84cb008b..c2a79660b1 100644 --- a/tests/post/steps/test_salt_api.py +++ b/tests/post/steps/test_salt_api.py @@ -15,11 +15,15 @@ def test_login_basic_auth_to_salt_api(host): pass + +@scenario('../features/salt_api.feature', + 'Login to SaltAPI using an admin ServiceAccount') @scenario('../features/salt_api.feature', 'Login to SaltAPI using a ServiceAccount') def test_login_bearer_auth_to_salt_api(host): pass + @scenario('../features/salt_api.feature', 'Login to SaltAPI using an incorrect password') def test_login_to_salt_api_using_an_incorrect_password(host, request): pass @@ -49,19 +53,25 @@ def login_salt_api_basic(host, username, password, version, context): context['salt-api'] = _salt_api_login(address, username, token, 'Basic') +@when("we login to SaltAPI with an admin ServiceAccount") +def login_salt_api_admin_sa(host, k8s_client, admin_sa, version, context): + sa_name, sa_namespace = admin_sa + address = _get_salt_api_address(host, version) + + context['salt-api'] = _login_salt_api_sa( + address, k8s_client, + sa_name, sa_namespace + ) + + @when(parsers.parse( "we login to SaltAPI with the ServiceAccount '{account_name}'")) -def login_salt_api_token(host, k8s_client, account_name, version, context): +def login_salt_api_system_sa(host, k8s_client, account_name, version, context): address = _get_salt_api_address(host, version) - service_account = k8s_client.read_namespaced_service_account( - name=account_name, namespace='kube-system' - ) - secret = k8s_client.read_namespaced_secret( - name=service_account.secrets[0].name, namespace='kube-system' - ) - token = base64.decodebytes(secret.data['token'].encode('utf-8')) - context['salt-api'] = _salt_api_login( - address, account_name, token, 'Bearer' + + context['salt-api'] = _login_salt_api_sa( + address, k8s_client, + account_name, 'kube-system' ) @@ -108,6 +118,19 @@ def have_perms(host, context, perms): # Helpers {{{ +def _login_salt_api_sa(address, k8s_client, sa_name, sa_namespace): + service_account = k8s_client.read_namespaced_service_account( + name=sa_name, namespace=sa_namespace + ) + secret = k8s_client.read_namespaced_secret( + name=service_account.secrets[0].name, namespace=sa_namespace + ) + token = base64.decodebytes(secret.data['token'].encode('utf-8')) + return _salt_api_login( + address, sa_name, token, 'Bearer' + ) + + def _get_salt_api_address(host, version): SALT_API_PORT = 4507 cmd_cidr = ' '.join([