diff --git a/cmd/local-user-authenticator/main.go b/cmd/local-user-authenticator/main.go index 7c1833ff..0dd3b42e 100644 --- a/cmd/local-user-authenticator/main.go +++ b/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, }, }, } diff --git a/cmd/local-user-authenticator/main_test.go b/cmd/local-user-authenticator/main_test.go index cee265ee..7bd9e64c 100644 --- a/cmd/local-user-authenticator/main_test.go +++ b/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, otherUser, otherUID, otherPassword, groups) - addSecretToFakeClientTracker(t, kubeClient, colonUser, colonUID, colonPassword, groups) - addSecretToFakeClientTracker(t, kubeClient, noGroupUser, noGroupUID, noGroupPassword, "") - addSecretToFakeClientTracker(t, kubeClient, oneGroupUser, oneGroupUID, oneGroupPassword, group0) - addSecretToFakeClientTracker(t, kubeClient, emptyPasswordUser, emptyPasswordUID, "", groups) + addSecretToFakeClientTracker(t, kubeClient, user, password, groups) + addSecretToFakeClientTracker(t, kubeClient, otherUser, otherPassword, groups) + addSecretToFakeClientTracker(t, kubeClient, colonUser, colonPassword, groups) + addSecretToFakeClientTracker(t, kubeClient, noGroupUser, noGroupPassword, "") + addSecretToFakeClientTracker(t, kubeClient, oneGroupUser, oneGroupPassword, group0) + 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, }, diff --git a/go.mod b/go.mod index d86141f4..b49227f5 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index e58f11bb..38309f92 100644 --- a/go.sum +++ b/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= diff --git a/internal/registry/credentialrequest/rest.go b/internal/registry/credentialrequest/rest.go index 9c9165d5..6e5b44b4 100644 --- a/internal/registry/credentialrequest/rest.go +++ b/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 := "" + 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}, ) } diff --git a/internal/registry/credentialrequest/rest_test.go b/internal/registry/credentialrequest/rest_test.go index 78d5dd73..f7a7cf7b 100644 --- a/internal/registry/credentialrequest/rest_test.go +++ b/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:,authenticated:false`) + requireOneLogStatement(r, logger, `"success" userID:,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() {