From b70c62a1b396bd1d62b0f42aff24f79ac41b844d Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Mon, 3 Aug 2020 17:29:55 -0500 Subject: [PATCH 1/2] Add a test case to TestSuccessfulLoginRequest to verify access as group. Signed-off-by: Matt Moyer --- test/integration/loginrequest_test.go | 97 +++++++++++++++++---------- 1 file changed, 63 insertions(+), 34 deletions(-) diff --git a/test/integration/loginrequest_test.go b/test/integration/loginrequest_test.go index 69abb0f5..53295adf 100644 --- a/test/integration/loginrequest_test.go +++ b/test/integration/loginrequest_test.go @@ -16,6 +16,7 @@ import ( rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" "github.com/suzerain-io/placeholder-name-api/pkg/apis/placeholder/v1alpha1" "github.com/suzerain-io/placeholder-name/test/library" @@ -36,6 +37,26 @@ func makeRequest(t *testing.T, spec v1alpha1.LoginRequestSpec) (*v1alpha1.LoginR }, metav1.CreateOptions{}) } +func addTestClusterRoleBinding(ctx context.Context, t *testing.T, adminClient kubernetes.Interface, binding *rbacv1.ClusterRoleBinding) { + _, err := adminClient.RbacV1().ClusterRoleBindings().Get(ctx, binding.Name, metav1.GetOptions{}) + if err != nil { + // "404 not found" errors are acceptable, but others would be unexpected + statusError, isStatus := err.(*errors.StatusError) + require.True(t, isStatus) + require.Equal(t, http.StatusNotFound, int(statusError.Status().Code)) + + _, err = adminClient.RbacV1().ClusterRoleBindings().Create(ctx, binding, metav1.CreateOptions{}) + require.NoError(t, err) + } + + t.Cleanup(func() { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + err = adminClient.RbacV1().ClusterRoleBindings().Delete(ctx, binding.Name, metav1.DeleteOptions{}) + require.NoError(t, err, "Test failed to clean up after itself") + }) +} + func TestSuccessfulLoginRequest(t *testing.T) { tmcClusterToken := os.Getenv("PLACEHOLDER_NAME_TMC_CLUSTER_TOKEN") require.NotEmptyf(t, tmcClusterToken, "must specify PLACEHOLDER_NAME_TMC_CLUSTER_TOKEN env var for integration tests") @@ -64,21 +85,24 @@ func TestSuccessfulLoginRequest(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) defer cancel() - const readonlyBindingName = "integration-test-user-readonly-role-binding" - + // Create a client using the admin kubeconfig. adminClient := library.NewClientset(t) - _, err = adminClient.RbacV1().ClusterRoleBindings().Get(ctx, readonlyBindingName, metav1.GetOptions{}) - if err != nil { - // "404 not found" errors are acceptable, but others would be unexpected - statusError, isStatus := err.(*errors.StatusError) - require.True(t, isStatus) - require.Equal(t, http.StatusNotFound, int(statusError.Status().Code)) - // Create a ClusterRoleBinding for this user only if one is not already found (so you can run tests more than once) - bindUserToReadonly := rbacv1.ClusterRoleBinding{ + // Create a client using the certificate from the LoginRequest. + clientWithCert := library.NewClientsetWithConfig( + t, + library.NewClientConfigWithCertAndKey( + t, + response.Status.Credential.ClientCertificateData, + response.Status.Credential.ClientKeyData, + ), + ) + + t.Run("access as user", func(t *testing.T) { + addTestClusterRoleBinding(ctx, t, adminClient, &rbacv1.ClusterRoleBinding{ TypeMeta: metav1.TypeMeta{}, ObjectMeta: metav1.ObjectMeta{ - Name: readonlyBindingName, + Name: "integration-test-user-readonly-role-binding", }, Subjects: []rbacv1.Subject{{ Kind: rbacv1.UserKind, @@ -90,32 +114,37 @@ func TestSuccessfulLoginRequest(t *testing.T) { APIGroup: rbacv1.GroupName, Name: "view", }, - } - _, err = adminClient.RbacV1().ClusterRoleBindings().Create(ctx, &bindUserToReadonly, metav1.CreateOptions{}) + }) + + // Use the client which is authenticated as the TMC user to list namespaces + listNamespaceResponse, err := clientWithCert.CoreV1().Namespaces().List(ctx, metav1.ListOptions{}) require.NoError(t, err) - } + require.NotEmpty(t, listNamespaceResponse.Items) + }) - defer func() { - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() - err = adminClient.RbacV1().ClusterRoleBindings().Delete(ctx, readonlyBindingName, metav1.DeleteOptions{}) - require.NoError(t, err, "Test failed to clean up after itself") - }() + t.Run("access as group", func(t *testing.T) { + addTestClusterRoleBinding(ctx, t, adminClient, &rbacv1.ClusterRoleBinding{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Name: "integration-test-group-readonly-role-binding", + }, + Subjects: []rbacv1.Subject{{ + Kind: rbacv1.GroupKind, + APIGroup: rbacv1.GroupName, + Name: "tmc:member", + }}, + RoleRef: rbacv1.RoleRef{ + Kind: "ClusterRole", + APIGroup: rbacv1.GroupName, + Name: "view", + }, + }) - // Create a client using the certificate from the LoginRequest - clientWithCert := library.NewClientsetWithConfig( - t, - library.NewClientConfigWithCertAndKey( - t, - response.Status.Credential.ClientCertificateData, - response.Status.Credential.ClientKeyData, - ), - ) - - // Use the client which is authenticated as the TMC user to list namespaces - listNamespaceResponse, err := clientWithCert.CoreV1().Namespaces().List(ctx, metav1.ListOptions{}) - require.NoError(t, err) - require.NotEmpty(t, listNamespaceResponse.Items) + // Use the client which is authenticated as the TMC group to list namespaces + listNamespaceResponse, err := clientWithCert.CoreV1().Namespaces().List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + require.NotEmpty(t, listNamespaceResponse.Items) + }) } func TestFailedLoginRequestWhenTheRequestIsValidButTheTokenDoesNotAuthenticateTheUser(t *testing.T) { From fdbc30365dd83f9360a1098112658229a70ae001 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Mon, 3 Aug 2020 17:31:18 -0500 Subject: [PATCH 2/2] Use the correct field when encoding groups into the certificate. Signed-off-by: Matt Moyer --- internal/registry/loginrequest/rest.go | 4 ++-- internal/registry/loginrequest/rest_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/registry/loginrequest/rest.go b/internal/registry/loginrequest/rest.go index fa5b95a1..c4bc4ed9 100644 --- a/internal/registry/loginrequest/rest.go +++ b/internal/registry/loginrequest/rest.go @@ -127,8 +127,8 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation certPEM, keyPEM, err := r.issuer.IssuePEM( pkix.Name{ - CommonName: authResponse.User.GetName(), - OrganizationalUnit: authResponse.User.GetGroups(), + CommonName: authResponse.User.GetName(), + Organization: authResponse.User.GetGroups(), }, []string{}, clientCertificateTTL, diff --git a/internal/registry/loginrequest/rest_test.go b/internal/registry/loginrequest/rest_test.go index add537be..0d89152b 100644 --- a/internal/registry/loginrequest/rest_test.go +++ b/internal/registry/loginrequest/rest_test.go @@ -141,8 +141,8 @@ func TestCreateSucceedsWhenGivenATokenAndTheWebhookAuthenticatesTheToken(t *test issuer := mockcertissuer.NewMockCertIssuer(ctrl) issuer.EXPECT().IssuePEM( pkix.Name{ - CommonName: "test-user", - OrganizationalUnit: []string{"test-group-1", "test-group-2"}}, + CommonName: "test-user", + Organization: []string{"test-group-1", "test-group-2"}}, []string{}, 1*time.Hour, ).Return([]byte("test-cert"), []byte("test-key"), nil)