More tests and more validations for create LoginRequest endpoint

- Mostly testing the way that the validation webhooks are called
- Also error when the auth webhook does not return user info, since we wouldn't know who you are in that case
This commit is contained in:
Ryan Richard 2020-07-24 11:00:29 -07:00
parent 6fe7a4c9dc
commit 9bfec08d90
2 changed files with 101 additions and 29 deletions

View File

@ -71,13 +71,15 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation
}
// let dynamic admission webhooks have a chance to validate (but not mutate) as well
// TODO Are we okay with admission webhooks being able to see tokens? Maybe strip token out before passing obj to createValidation.
// Since we are an aggregated API, we should investigate to see if the kube API server is already invoking admission hooks for us.
// Even if it is, its okay to call it again here. If the kube API server is already calling the webhooks and passing
// the token, then there is probably no reason for us to avoid passing the token when we call the webhooks here, since
// they already got the token.
// TODO Since we are an aggregated API, we should investigate to see if the kube API server is already invoking admission hooks for us.
// Even if it is, its okay to call it again here. However, if the kube API server is already calling the webhooks and passing
// the token, then there is probably no reason for us to avoid passing the token when we call the webhooks here, since
// they already got the token.
if createValidation != nil {
if err := createValidation(ctx, obj.DeepCopyObject()); err != nil {
requestForValidation := obj.DeepCopyObject()
loginRequestCopy, _ := requestForValidation.(*placeholderapi.LoginRequest)
loginRequestCopy.Spec.Token.Value = ""
if err := createValidation(ctx, requestForValidation); err != nil {
return nil, err
}
}
@ -110,7 +112,7 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation
}
var out *placeholderapi.LoginRequest
if authenticated {
if authenticated && authResponse.User != nil && authResponse.User.GetName() != "" {
out = successfulResponse(authResponse)
} else {
out = failureResponse()

View File

@ -65,7 +65,7 @@ func callCreate(ctx context.Context, storage *REST, loginRequest *placeholderapi
}
func validLoginRequest() *placeholderapi.LoginRequest {
return validLoginRequestWithToken("a token")
return validLoginRequestWithToken("some token")
}
func validLoginRequestWithToken(token string) *placeholderapi.LoginRequest {
@ -85,6 +85,15 @@ func loginRequest(spec placeholderapi.LoginRequestSpec) *placeholderapi.LoginReq
}
}
func webhookSuccessResponse() *authenticator.Response {
return &authenticator.Response{User: &user.DefaultInfo{
Name: "some-user",
UID: "",
Groups: []string{},
Extra: nil,
}}
}
func requireAPIError(t *testing.T, response runtime.Object, err error, expectedErrorTypeChecker func(err error) bool, expectedErrorMessage string) {
t.Helper()
require.Nil(t, response)
@ -94,8 +103,14 @@ func requireAPIError(t *testing.T, response runtime.Object, err error, expectedE
require.Contains(t, status.Status().Message, expectedErrorMessage)
}
func emptyWebhookSuccessResponse() *authenticator.Response {
return &authenticator.Response{User: &user.DefaultInfo{}}
func requireSuccessfulResponseWithAuthenticationFailureMessage(t *testing.T, err error, response runtime.Object) {
require.NoError(t, err)
require.Equal(t, response, &placeholderapi.LoginRequest{
Status: placeholderapi.LoginRequestStatus{
Credential: nil,
Message: "authentication failed",
},
})
}
func TestCreateSucceedsWhenGivenATokenAndTheWebhookAuthenticatesTheToken(t *testing.T) {
@ -141,13 +156,7 @@ func TestCreateSucceedsWithAnUnauthenticatedStatusWhenGivenATokenAndTheWebhookDo
response, err := callCreate(context.Background(), storage, validLoginRequestWithToken(requestToken))
require.NoError(t, err)
require.Equal(t, response, &placeholderapi.LoginRequest{
Status: placeholderapi.LoginRequestStatus{
Credential: nil,
Message: "authentication failed",
},
})
requireSuccessfulResponseWithAuthenticationFailureMessage(t, err, response)
require.Equal(t, requestToken, webhook.calledWithToken)
}
@ -159,18 +168,38 @@ func TestCreateSucceedsWithAnUnauthenticatedStatusWhenWebhookFails(t *testing.T)
response, err := callCreate(context.Background(), storage, validLoginRequest())
require.NoError(t, err)
require.Equal(t, response, &placeholderapi.LoginRequest{
Status: placeholderapi.LoginRequestStatus{
Credential: nil,
Message: "authentication failed",
requireSuccessfulResponseWithAuthenticationFailureMessage(t, err, response)
}
func TestCreateSucceedsWithAnUnauthenticatedStatusWhenWebhookDoesNotReturnAnyUserInfo(t *testing.T) {
webhook := FakeToken{
returnResponse: &authenticator.Response{},
}
storage := NewREST(&webhook)
response, err := callCreate(context.Background(), storage, validLoginRequest())
requireSuccessfulResponseWithAuthenticationFailureMessage(t, err, response)
}
func TestCreateSucceedsWithAnUnauthenticatedStatusWhenWebhookReturnsAnEmptyUsername(t *testing.T) {
webhook := FakeToken{
returnResponse: &authenticator.Response{
User: &user.DefaultInfo{
Name: "",
},
},
})
}
storage := NewREST(&webhook)
response, err := callCreate(context.Background(), storage, validLoginRequest())
requireSuccessfulResponseWithAuthenticationFailureMessage(t, err, response)
}
func TestCreateDoesNotPassAdditionalContextInfoToTheWebhook(t *testing.T) {
webhook := FakeToken{
returnResponse: emptyWebhookSuccessResponse(),
returnResponse: webhookSuccessResponse(),
}
storage := NewREST(&webhook)
ctx := context.WithValue(context.Background(), contextKey{}, "context-value")
@ -244,13 +273,54 @@ func TestCreateFailsWhenValidationFails(t *testing.T) {
func(ctx context.Context, obj runtime.Object) error {
return fmt.Errorf("some validation error")
},
&metav1.CreateOptions{
DryRun: []string{},
})
&metav1.CreateOptions{})
require.Nil(t, response)
require.EqualError(t, err, "some validation error")
}
func TestCreateDoesNotAllowValidationFunctionToMutateRequest(t *testing.T) {
webhook := FakeToken{
returnResponse: webhookSuccessResponse(),
}
storage := NewREST(&webhook)
requestToken := "a token"
response, err := storage.Create(
context.Background(),
validLoginRequestWithToken(requestToken),
func(ctx context.Context, obj runtime.Object) error {
loginRequest, _ := obj.(*placeholderapi.LoginRequest)
loginRequest.Spec.Token.Value = "foobaz"
return nil
},
&metav1.CreateOptions{})
require.NoError(t, err)
require.NotEmpty(t, response)
require.Equal(t, requestToken, webhook.calledWithToken) // i.e. not called with foobaz
}
func TestCreateDoesNotAllowValidationFunctionToSeeTheActualRequestToken(t *testing.T) {
webhook := FakeToken{
returnResponse: webhookSuccessResponse(),
}
storage := NewREST(&webhook)
validationFunctionWasCalled := false
var validationFunctionSawTokenValue string
response, err := storage.Create(
context.Background(),
validLoginRequest(),
func(ctx context.Context, obj runtime.Object) error {
loginRequest, _ := obj.(*placeholderapi.LoginRequest)
validationFunctionWasCalled = true
validationFunctionSawTokenValue = loginRequest.Spec.Token.Value
return nil
},
&metav1.CreateOptions{})
require.NoError(t, err)
require.NotEmpty(t, response)
require.True(t, validationFunctionWasCalled)
require.Empty(t, validationFunctionSawTokenValue)
}
func TestCreateFailsWhenRequestOptionsDryRunIsNotEmpty(t *testing.T) {
response, err := NewREST(&FakeToken{}).Create(
genericapirequest.NewContext(),
@ -269,7 +339,7 @@ func TestCreateCancelsTheWebhookInvocationWhenTheCallToCreateIsCancelledItself(t
webhook := FakeToken{
timeout: time.Second * 2,
webhookStartedRunningNotificationChan: webhookStartedRunningNotificationChan,
returnResponse: emptyWebhookSuccessResponse(),
returnResponse: webhookSuccessResponse(),
}
storage := NewREST(&webhook)
ctx, cancel := context.WithCancel(context.Background())
@ -296,7 +366,7 @@ func TestCreateAllowsTheWebhookInvocationToFinishWhenTheCallToCreateIsNotCancell
webhook := FakeToken{
timeout: 0,
webhookStartedRunningNotificationChan: webhookStartedRunningNotificationChan,
returnResponse: emptyWebhookSuccessResponse(),
returnResponse: webhookSuccessResponse(),
}
storage := NewREST(&webhook)
ctx, cancel := context.WithCancel(context.Background())