Merge pull request #39 from mattmoyer/fix-certificate-group-field

Fix group identity encoding in client certificates.

internal/registry/loginrequest/rest.go

@@ -128,7 +128,7 @@ 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(),
+			Organization: authResponse.User.GetGroups(),
 		},
 		[]string{},
 		clientCertificateTTL,

internal/registry/loginrequest/rest_test.go

@@ -142,7 +142,7 @@ func TestCreateSucceedsWhenGivenATokenAndTheWebhookAuthenticatesTheToken(t *test
 	issuer.EXPECT().IssuePEM(
 		pkix.Name{
 			CommonName:   "test-user",
-			OrganizationalUnit: []string{"test-group-1", "test-group-2"}},
+			Organization: []string{"test-group-1", "test-group-2"}},
 		[]string{},
 		1*time.Hour,
 	).Return([]byte("test-cert"), []byte("test-key"), nil)

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{})
-		require.NoError(t, err)
-	}
-
-	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")
-	}()
-
-	// 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)
+	})
+
+	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",
+			},
+		})
+
+		// 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) {