Merge branch 'initial_ldap_group_support' into ldap_starttls

cmd/local-user-authenticator/main.go

@@ -150,7 +150,7 @@ func (w *webhook) ServeHTTP(rsp http.ResponseWriter, req *http.Request) {
 	}
 
 	plog.Debug("successful authentication")
-	respondWithAuthenticated(rsp, secret.ObjectMeta.Name, string(secret.UID), groups)
+	respondWithAuthenticated(rsp, secret.ObjectMeta.Name, groups)
 }
 
 func getUsernameAndPasswordFromRequest(rsp http.ResponseWriter, req *http.Request) (string, string, error) {
@@ -255,7 +255,7 @@ func respondWithUnauthenticated(rsp http.ResponseWriter) {
 
 func respondWithAuthenticated(
 	rsp http.ResponseWriter,
-	username, uid string,
+	username string,
 	groups []string,
 ) {
 	rsp.Header().Add("Content-Type", "application/json")
@@ -269,7 +269,6 @@ func respondWithAuthenticated(
 			User: authenticationv1beta1.UserInfo{
 				Username: username,
 				Groups:   groups,
-				UID:      uid,
 			},
 		},
 	}

cmd/local-user-authenticator/main_test.go

@@ -25,7 +25,6 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
-	"k8s.io/apimachinery/pkg/types"
 	kubeinformers "k8s.io/client-go/informers"
 	corev1informers "k8s.io/client-go/informers/core/v1"
 	"k8s.io/client-go/kubernetes"
@@ -45,8 +44,6 @@ func TestWebhook(t *testing.T) {
 
 	user, otherUser, colonUser, noGroupUser, oneGroupUser, passwordUndefinedUser, emptyPasswordUser, invalidPasswordHashUser, undefinedGroupsUser :=
 		"some-user", "other-user", "colon-user", "no-group-user", "one-group-user", "password-undefined-user", "empty-password-user", "invalid-password-hash-user", "undefined-groups-user"
-	uid, otherUID, colonUID, noGroupUID, oneGroupUID, passwordUndefinedUID, emptyPasswordUID, invalidPasswordHashUID, undefinedGroupsUID :=
-		"some-uid", "other-uid", "colon-uid", "no-group-uid", "one-group-uid", "password-undefined-uid", "empty-password-uid", "invalid-password-hash-uid", "undefined-groups-uid"
 	password, otherPassword, colonPassword, noGroupPassword, oneGroupPassword, undefinedGroupsPassword :=
 		"some-password", "other-password", "some-:-password", "no-group-password", "one-group-password", "undefined-groups-password"
 
@@ -54,16 +51,15 @@ func TestWebhook(t *testing.T) {
 	groups := group0 + " , " + group1
 
 	kubeClient := kubernetesfake.NewSimpleClientset()
-	addSecretToFakeClientTracker(t, kubeClient, user, uid, password, groups)
+	addSecretToFakeClientTracker(t, kubeClient, user, password, groups)
-	addSecretToFakeClientTracker(t, kubeClient, otherUser, otherUID, otherPassword, groups)
+	addSecretToFakeClientTracker(t, kubeClient, otherUser, otherPassword, groups)
-	addSecretToFakeClientTracker(t, kubeClient, colonUser, colonUID, colonPassword, groups)
+	addSecretToFakeClientTracker(t, kubeClient, colonUser, colonPassword, groups)
-	addSecretToFakeClientTracker(t, kubeClient, noGroupUser, noGroupUID, noGroupPassword, "")
+	addSecretToFakeClientTracker(t, kubeClient, noGroupUser, noGroupPassword, "")
-	addSecretToFakeClientTracker(t, kubeClient, oneGroupUser, oneGroupUID, oneGroupPassword, group0)
+	addSecretToFakeClientTracker(t, kubeClient, oneGroupUser, oneGroupPassword, group0)
-	addSecretToFakeClientTracker(t, kubeClient, emptyPasswordUser, emptyPasswordUID, "", groups)
+	addSecretToFakeClientTracker(t, kubeClient, emptyPasswordUser, "", groups)
 
 	require.NoError(t, kubeClient.Tracker().Add(&corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
-			UID:       types.UID(passwordUndefinedUID),
 			Name:      passwordUndefinedUser,
 			Namespace: namespace,
 		},
@@ -77,7 +73,6 @@ func TestWebhook(t *testing.T) {
 
 	require.NoError(t, kubeClient.Tracker().Add(&corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
-			UID:       types.UID(undefinedGroupsUID),
 			Name:      undefinedGroupsUser,
 			Namespace: namespace,
 		},
@@ -88,7 +83,6 @@ func TestWebhook(t *testing.T) {
 
 	require.NoError(t, kubeClient.Tracker().Add(&corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
-			UID:       types.UID(invalidPasswordHashUID),
 			Name:      invalidPasswordHashUser,
 			Namespace: namespace,
 		},
@@ -135,7 +129,7 @@ func TestWebhook(t *testing.T) {
 			body:        func() (io.ReadCloser, error) { return newTokenReviewBody(user + ":" + password) },
 			wantStatus:  http.StatusOK,
 			wantHeaders: map[string][]string{"Content-Type": {"application/json"}},
-			wantBody:    authenticatedResponseJSON(user, uid, []string{group0, group1}),
+			wantBody:    authenticatedResponseJSON(user, []string{group0, group1}),
 		},
 		{
 			name:        "success for a user who belongs to one groups",
@@ -145,7 +139,7 @@ func TestWebhook(t *testing.T) {
 			body:        func() (io.ReadCloser, error) { return newTokenReviewBody(oneGroupUser + ":" + oneGroupPassword) },
 			wantStatus:  http.StatusOK,
 			wantHeaders: map[string][]string{"Content-Type": {"application/json"}},
-			wantBody:    authenticatedResponseJSON(oneGroupUser, oneGroupUID, []string{group0}),
+			wantBody:    authenticatedResponseJSON(oneGroupUser, []string{group0}),
 		},
 		{
 			name:        "success for a user who belongs to no groups",
@@ -155,7 +149,7 @@ func TestWebhook(t *testing.T) {
 			body:        func() (io.ReadCloser, error) { return newTokenReviewBody(noGroupUser + ":" + noGroupPassword) },
 			wantStatus:  http.StatusOK,
 			wantHeaders: map[string][]string{"Content-Type": {"application/json"}},
-			wantBody:    authenticatedResponseJSON(noGroupUser, noGroupUID, nil),
+			wantBody:    authenticatedResponseJSON(noGroupUser, nil),
 		},
 		{
 			name:        "wrong username for password",
@@ -197,7 +191,7 @@ func TestWebhook(t *testing.T) {
 			},
 			wantStatus:  http.StatusOK,
 			wantHeaders: map[string][]string{"Content-Type": {"application/json"}},
-			wantBody:    authenticatedResponseJSON(undefinedGroupsUser, undefinedGroupsUID, nil),
+			wantBody:    authenticatedResponseJSON(undefinedGroupsUser, nil),
 		},
 		{
 			name:        "when a user has empty string as their password",
@@ -255,7 +249,7 @@ func TestWebhook(t *testing.T) {
 			body:        func() (io.ReadCloser, error) { return newTokenReviewBody(colonUser + ":" + colonPassword) },
 			wantStatus:  http.StatusOK,
 			wantHeaders: map[string][]string{"Content-Type": {"application/json"}},
-			wantBody:    authenticatedResponseJSON(colonUser, colonUID, []string{group0, group1}),
+			wantBody:    authenticatedResponseJSON(colonUser, []string{group0, group1}),
 		},
 		{
 			name:    "bad TokenReview group",
@@ -357,7 +351,7 @@ func TestWebhook(t *testing.T) {
 			body:        func() (io.ReadCloser, error) { return newTokenReviewBody(user + ":" + password) },
 			wantStatus:  http.StatusOK,
 			wantHeaders: map[string][]string{"Content-Type": {"application/json"}},
-			wantBody:    authenticatedResponseJSON(user, uid, []string{group0, group1}),
+			wantBody:    authenticatedResponseJSON(user, []string{group0, group1}),
 		},
 		{
 			name:   "success when there are multiple accepts and one of them is */*",
@@ -370,7 +364,7 @@ func TestWebhook(t *testing.T) {
 			body:        func() (io.ReadCloser, error) { return newTokenReviewBody(user + ":" + password) },
 			wantStatus:  http.StatusOK,
 			wantHeaders: map[string][]string{"Content-Type": {"application/json"}},
-			wantBody:    authenticatedResponseJSON(user, uid, []string{group0, group1}),
+			wantBody:    authenticatedResponseJSON(user, []string{group0, group1}),
 		},
 		{
 			name:   "success when there are multiple accepts and one of them is application/*",
@@ -383,7 +377,7 @@ func TestWebhook(t *testing.T) {
 			body:        func() (io.ReadCloser, error) { return newTokenReviewBody(user + ":" + password) },
 			wantStatus:  http.StatusOK,
 			wantHeaders: map[string][]string{"Content-Type": {"application/json"}},
-			wantBody:    authenticatedResponseJSON(user, uid, []string{group0, group1}),
+			wantBody:    authenticatedResponseJSON(user, []string{group0, group1}),
 		},
 		{
 			name:       "bad body",
@@ -538,7 +532,7 @@ func unauthenticatedResponseJSON() *authenticationv1beta1.TokenReview {
 	}
 }
 
-func authenticatedResponseJSON(user, uid string, groups []string) *authenticationv1beta1.TokenReview {
+func authenticatedResponseJSON(user string, groups []string) *authenticationv1beta1.TokenReview {
 	return &authenticationv1beta1.TokenReview{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       "TokenReview",
@@ -549,19 +543,17 @@ func authenticatedResponseJSON(user, uid string, groups []string) *authenticatio
 			User: authenticationv1beta1.UserInfo{
 				Username: user,
 				Groups:   groups,
-				UID:      uid,
 			},
 		},
 	}
 }
 
-func addSecretToFakeClientTracker(t *testing.T, kubeClient *kubernetesfake.Clientset, username, uid, password, groups string) {
+func addSecretToFakeClientTracker(t *testing.T, kubeClient *kubernetesfake.Clientset, username, password, groups string) {
 	passwordHash, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.MinCost)
 	require.NoError(t, err)
 
 	secret := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
-			UID:       types.UID(uid),
 			Name:      username,
 			Namespace: namespace,
 		},

go.mod

@@ -6,7 +6,7 @@ require (
 	cloud.google.com/go v0.60.0 // indirect
 	github.com/MakeNowJust/heredoc/v2 v2.0.1
 	github.com/coreos/go-oidc/v3 v3.0.0
-	github.com/creack/pty v1.1.11
+	github.com/creack/pty v1.1.12
 	github.com/davecgh/go-spew v1.1.1
 	github.com/go-ldap/ldap/v3 v3.3.0
 	github.com/go-logr/logr v0.4.0

go.sum

@@ -148,8 +148,9 @@ github.com/cpuguy83/go-md2man/v2 v2.0.0 h1:EoUDS0afbrsXAZ9YQ9jdu/mZ2sXgT1/2yyNng
 github.com/cpuguy83/go-md2man/v2 v2.0.0/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsrgA7czyZG/E6dU=
 github.com/creack/pty v1.1.7/go.mod h1:lj5s0c3V2DBrqTV7llrYr5NG6My20zk30Fl46Y7DoTY=
 github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
-github.com/creack/pty v1.1.11 h1:07n33Z8lZxZ2qwegKbObQohDhXDQxiMMz1NOUGYlesw=
 github.com/creack/pty v1.1.11/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
+github.com/creack/pty v1.1.12 h1:2QLiUCEbsI13vUyWH6026g0o4u7vxgg2hLUc+D7FPFU=
+github.com/creack/pty v1.1.12/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
 github.com/cucumber/godog v0.8.1/go.mod h1:vSh3r/lM+psC1BPXvdkSEuNjmXfpVqrMGYAElF6hxnA=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=

internal/registry/credentialrequest/rest.go

@@ -101,7 +101,7 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation
 		traceFailureWithError(t, "token authentication", err)
 		return failureResponse(), nil
 	}
-	if userInfo == nil || userInfo.GetName() == "" {
+	if ok := isUserInfoValid(userInfo); !ok {
 		traceSuccess(t, userInfo, false)
 		return failureResponse(), nil
 	}
@@ -169,13 +169,29 @@ func validateRequest(ctx context.Context, obj runtime.Object, createValidation r
 	return credentialRequest, nil
 }
 
+func isUserInfoValid(userInfo user.Info) bool {
+	switch {
+	case userInfo == nil, // must be non-nil
+		len(userInfo.GetName()) == 0,  // must have a username, groups are optional
+		len(userInfo.GetUID()) != 0,   // certs cannot assert UID
+		len(userInfo.GetExtra()) != 0: // certs cannot assert extra
+		return false
+
+	default:
+		return true
+	}
+}
+
 func traceSuccess(t *trace.Trace, userInfo user.Info, authenticated bool) {
 	userID := "<none>"
+	hasExtra := false
 	if userInfo != nil {
 		userID = userInfo.GetUID()
+		hasExtra = len(userInfo.GetExtra()) > 0
 	}
 	t.Step("success",
 		trace.Field{Key: "userID", Value: userID},
+		trace.Field{Key: "hasExtra", Value: hasExtra},
 		trace.Field{Key: "authenticated", Value: authenticated},
 	)
 }

internal/registry/credentialrequest/rest_test.go

@@ -86,7 +86,6 @@ func TestCreate(t *testing.T) {
 			requestAuthenticator.EXPECT().AuthenticateTokenCredentialRequest(gomock.Any(), req).
 				Return(&user.DefaultInfo{
 					Name:   "test-user",
-					UID:    "test-user-uid",
 					Groups: []string{"test-group-1", "test-group-2"},
 				}, nil)
 
@@ -118,7 +117,7 @@ func TestCreate(t *testing.T) {
 					},
 				},
 			})
-			requireOneLogStatement(r, logger, `"success" userID:test-user-uid,authenticated:true`)
+			requireOneLogStatement(r, logger, `"success" userID:,hasExtra:false,authenticated:true`)
 		})
 
 		it("CreateFailsWithValidTokenWhenCertIssuerFails", func() {
@@ -154,7 +153,7 @@ func TestCreate(t *testing.T) {
 			response, err := callCreate(context.Background(), storage, req)
 
 			requireSuccessfulResponseWithAuthenticationFailureMessage(t, err, response)
-			requireOneLogStatement(r, logger, `"success" userID:<none>,authenticated:false`)
+			requireOneLogStatement(r, logger, `"success" userID:<none>,hasExtra:false,authenticated:false`)
 		})
 
 		it("CreateSucceedsWithAnUnauthenticatedStatusWhenWebhookFails", func() {
@@ -184,7 +183,45 @@ func TestCreate(t *testing.T) {
 			response, err := callCreate(context.Background(), storage, req)
 
 			requireSuccessfulResponseWithAuthenticationFailureMessage(t, err, response)
-			requireOneLogStatement(r, logger, `"success" userID:,authenticated:false`)
+			requireOneLogStatement(r, logger, `"success" userID:,hasExtra:false,authenticated:false`)
+		})
+
+		it("CreateSucceedsWithAnUnauthenticatedStatusWhenWebhookReturnsAUserWithUID", func() {
+			req := validCredentialRequest()
+
+			requestAuthenticator := credentialrequestmocks.NewMockTokenCredentialRequestAuthenticator(ctrl)
+			requestAuthenticator.EXPECT().AuthenticateTokenCredentialRequest(gomock.Any(), req).
+				Return(&user.DefaultInfo{
+					Name:   "test-user",
+					UID:    "test-uid",
+					Groups: []string{"test-group-1", "test-group-2"},
+				}, nil)
+
+			storage := NewREST(requestAuthenticator, nil, schema.GroupResource{})
+
+			response, err := callCreate(context.Background(), storage, req)
+
+			requireSuccessfulResponseWithAuthenticationFailureMessage(t, err, response)
+			requireOneLogStatement(r, logger, `"success" userID:test-uid,hasExtra:false,authenticated:false`)
+		})
+
+		it("CreateSucceedsWithAnUnauthenticatedStatusWhenWebhookReturnsAUserWithExtra", func() {
+			req := validCredentialRequest()
+
+			requestAuthenticator := credentialrequestmocks.NewMockTokenCredentialRequestAuthenticator(ctrl)
+			requestAuthenticator.EXPECT().AuthenticateTokenCredentialRequest(gomock.Any(), req).
+				Return(&user.DefaultInfo{
+					Name:   "test-user",
+					Groups: []string{"test-group-1", "test-group-2"},
+					Extra:  map[string][]string{"test-key": {"test-val-1", "test-val-2"}},
+				}, nil)
+
+			storage := NewREST(requestAuthenticator, nil, schema.GroupResource{})
+
+			response, err := callCreate(context.Background(), storage, req)
+
+			requireSuccessfulResponseWithAuthenticationFailureMessage(t, err, response)
+			requireOneLogStatement(r, logger, `"success" userID:,hasExtra:true,authenticated:false`)
 		})
 
 		it("CreateFailsWhenGivenTheWrongInputType", func() {