cred req: disallow lossy user info translations

Signed-off-by: Monis Khan <mok@vmware.com>
This commit is contained in:
Monis Khan 2021-05-03 14:06:49 -04:00
parent 99099fd32f
commit 35479e2978
No known key found for this signature in database
GPG Key ID: 52C90ADA01B269B8
2 changed files with 58 additions and 5 deletions

View File

@ -101,7 +101,7 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation
traceFailureWithError(t, "token authentication", err) traceFailureWithError(t, "token authentication", err)
return failureResponse(), nil return failureResponse(), nil
} }
if userInfo == nil || userInfo.GetName() == "" { if ok := isUserInfoValid(userInfo); !ok {
traceSuccess(t, userInfo, false) traceSuccess(t, userInfo, false)
return failureResponse(), nil return failureResponse(), nil
} }
@ -169,13 +169,29 @@ func validateRequest(ctx context.Context, obj runtime.Object, createValidation r
return credentialRequest, nil 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) { func traceSuccess(t *trace.Trace, userInfo user.Info, authenticated bool) {
userID := "<none>" userID := "<none>"
hasExtra := false
if userInfo != nil { if userInfo != nil {
userID = userInfo.GetUID() userID = userInfo.GetUID()
hasExtra = len(userInfo.GetExtra()) > 0
} }
t.Step("success", t.Step("success",
trace.Field{Key: "userID", Value: userID}, trace.Field{Key: "userID", Value: userID},
trace.Field{Key: "hasExtra", Value: hasExtra},
trace.Field{Key: "authenticated", Value: authenticated}, trace.Field{Key: "authenticated", Value: authenticated},
) )
} }

View File

@ -86,7 +86,6 @@ func TestCreate(t *testing.T) {
requestAuthenticator.EXPECT().AuthenticateTokenCredentialRequest(gomock.Any(), req). requestAuthenticator.EXPECT().AuthenticateTokenCredentialRequest(gomock.Any(), req).
Return(&user.DefaultInfo{ Return(&user.DefaultInfo{
Name: "test-user", Name: "test-user",
UID: "test-user-uid",
Groups: []string{"test-group-1", "test-group-2"}, Groups: []string{"test-group-1", "test-group-2"},
}, nil) }, 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() { it("CreateFailsWithValidTokenWhenCertIssuerFails", func() {
@ -154,7 +153,7 @@ func TestCreate(t *testing.T) {
response, err := callCreate(context.Background(), storage, req) response, err := callCreate(context.Background(), storage, req)
requireSuccessfulResponseWithAuthenticationFailureMessage(t, err, response) 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() { it("CreateSucceedsWithAnUnauthenticatedStatusWhenWebhookFails", func() {
@ -184,7 +183,45 @@ func TestCreate(t *testing.T) {
response, err := callCreate(context.Background(), storage, req) response, err := callCreate(context.Background(), storage, req)
requireSuccessfulResponseWithAuthenticationFailureMessage(t, err, response) 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() { it("CreateFailsWhenGivenTheWrongInputType", func() {