diff --git a/go.mod b/go.mod index f738f5b4..f65ac314 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,7 @@ require ( github.com/google/go-cmp v0.4.0 github.com/spf13/cobra v1.0.0 github.com/stretchr/testify v1.6.1 - github.com/suzerain-io/placeholder-name-api v0.0.0-20200714184318-8ad91581433a + github.com/suzerain-io/placeholder-name-api v0.0.0-20200724000517-dc602fd8d75e github.com/suzerain-io/placeholder-name-client-go v0.0.0-20200714203950-a414963b4f95 golang.org/x/tools v0.0.0-20200707134715-9e0a013e855f // indirect k8s.io/api v0.19.0-rc.0 diff --git a/go.sum b/go.sum index d0a134d0..98126ac4 100644 --- a/go.sum +++ b/go.sum @@ -521,6 +521,8 @@ github.com/subosito/gotenv v1.2.0 h1:Slr1R9HxAlEKefgq5jn9U+DnETlIUa6HfgEzj0g5d7s github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw= github.com/suzerain-io/placeholder-name-api v0.0.0-20200714184318-8ad91581433a h1:ycG6TufZM7ZDrgBklkXEMauvSyh44JQDvSUdJawAjGA= github.com/suzerain-io/placeholder-name-api v0.0.0-20200714184318-8ad91581433a/go.mod h1:bNHheAnmAISdW/ZYTnhCmg8QQKwA5WD64ZvPdsTrWjw= +github.com/suzerain-io/placeholder-name-api v0.0.0-20200724000517-dc602fd8d75e h1:PavRTcjOk0SUtIRUY4OaFXB3CVYFLTIlxLptUawLB1o= +github.com/suzerain-io/placeholder-name-api v0.0.0-20200724000517-dc602fd8d75e/go.mod h1:bNHheAnmAISdW/ZYTnhCmg8QQKwA5WD64ZvPdsTrWjw= github.com/suzerain-io/placeholder-name-client-go v0.0.0-20200714203950-a414963b4f95 h1:O1aoszdMJEekLXD7pNWP23vq0g8eXLp40AQgY8Hj+Sw= github.com/suzerain-io/placeholder-name-client-go v0.0.0-20200714203950-a414963b4f95/go.mod h1:NwirXEgd1VaA+6R0W7PYL/ae6wfPT3vA+tu7POAArjU= github.com/tdakkota/asciicheck v0.0.0-20200416190851-d7f85be797a2 h1:Xr9gkxfOP0KQWXKNqmwe8vEeSUiUj4Rlee9CMVX2ZUQ= diff --git a/pkg/registry/loginrequest/rest.go b/pkg/registry/loginrequest/rest.go index ca366263..2d8d4bd6 100644 --- a/pkg/registry/loginrequest/rest.go +++ b/pkg/registry/loginrequest/rest.go @@ -16,6 +16,7 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apiserver/pkg/authentication/authenticator" "k8s.io/apiserver/pkg/registry/rest" + "k8s.io/klog/v2" placeholderapi "github.com/suzerain-io/placeholder-name-api/pkg/apis/placeholder" ) @@ -53,7 +54,16 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation // TODO refactor all validation checks into a validation function in another package (e.g. see subjectaccessreqview api in k8s) - // TODO also validate .spec.type + if len(loginRequest.Spec.Type) == 0 { + errs := field.ErrorList{field.Required(field.NewPath("spec", "type"), "type must be supplied")} + return nil, apierrors.NewInvalid(placeholderapi.Kind(loginRequest.Kind), loginRequest.Name, errs) + } + + if loginRequest.Spec.Type != placeholderapi.TokenLoginCredentialType { + errs := field.ErrorList{field.Invalid(field.NewPath("spec", "type"), loginRequest.Spec.Type, "unrecognized type")} + return nil, apierrors.NewInvalid(placeholderapi.Kind(loginRequest.Kind), loginRequest.Name, errs) + } + token := loginRequest.Spec.Token if token == nil || len(token.Value) == 0 { errs := field.ErrorList{field.Required(field.NewPath("spec", "token", "value"), "token must be supplied")} @@ -93,22 +103,44 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation } }() - // TODO do the actual business logic of this endpoint here - - _, _, err := r.webhook.AuthenticateToken(cancelCtx, token.Value) + authResponse, authenticated, err := r.webhook.AuthenticateToken(cancelCtx, token.Value) if err != nil { - return nil, fmt.Errorf("authenticate token failed: %w", err) + klog.Warningf("webhook authentication failure: %v", err) + return failureResponse(), nil } - // make a new object so that we do not return the original token in the response - out := &placeholderapi.LoginRequest{ - Status: placeholderapi.LoginRequestStatus{ - ExpirationTimestamp: nil, - Token: "snorlax", - ClientCertificateData: "", - ClientKeyData: "", - }, + var out *placeholderapi.LoginRequest + if authenticated { + out = successfulResponse(authResponse) + } else { + out = failureResponse() } return out, nil } + +func successfulResponse(authResponse *authenticator.Response) *placeholderapi.LoginRequest { + return &placeholderapi.LoginRequest{ + Status: placeholderapi.LoginRequestStatus{ + Credential: &placeholderapi.LoginRequestCredential{ + ExpirationTimestamp: nil, + Token: "snorlax", + ClientCertificateData: "", + ClientKeyData: "", + }, + User: &placeholderapi.User{ + Name: authResponse.User.GetName(), + Groups: authResponse.User.GetGroups(), + }, + }, + } +} + +func failureResponse() *placeholderapi.LoginRequest { + return &placeholderapi.LoginRequest{ + Status: placeholderapi.LoginRequestStatus{ + Credential: nil, + Message: "authentication failed", + }, + } +} diff --git a/pkg/registry/loginrequest/rest_test.go b/pkg/registry/loginrequest/rest_test.go index 624a3834..b21a8619 100644 --- a/pkg/registry/loginrequest/rest_test.go +++ b/pkg/registry/loginrequest/rest_test.go @@ -11,12 +11,12 @@ import ( "testing" "time" - apierrors "k8s.io/apimachinery/pkg/api/errors" - "github.com/stretchr/testify/require" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/authentication/authenticator" + "k8s.io/apiserver/pkg/authentication/user" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/rest" @@ -32,6 +32,8 @@ type FakeToken struct { reachedTimeout bool cancelled bool webhookStartedRunningNotificationChan chan bool + returnResponse *authenticator.Response + returnUnauthenticated bool returnErr error } @@ -48,7 +50,7 @@ func (f *FakeToken) AuthenticateToken(ctx context.Context, token string) (*authe case <-ctx.Done(): f.cancelled = true } - return &authenticator.Response{}, true, f.returnErr + return f.returnResponse, !f.returnUnauthenticated, f.returnErr } func callCreate(ctx context.Context, storage *REST, loginRequest *placeholderapi.LoginRequest) (runtime.Object, error) { @@ -62,9 +64,13 @@ func callCreate(ctx context.Context, storage *REST, loginRequest *placeholderapi } func validLoginRequest() *placeholderapi.LoginRequest { + return validLoginRequestWithToken("a token") +} + +func validLoginRequestWithToken(token string) *placeholderapi.LoginRequest { return loginRequest(placeholderapi.LoginRequestSpec{ Type: placeholderapi.TokenLoginCredentialType, - Token: &placeholderapi.LoginRequestTokenCredential{Value: "a token"}, + Token: &placeholderapi.LoginRequestTokenCredential{Value: token}, }) } @@ -78,29 +84,93 @@ func loginRequest(spec placeholderapi.LoginRequestSpec) *placeholderapi.LoginReq } } -func TestCreateSucceedsWhenGivenAToken(t *testing.T) { - webhook := FakeToken{} +func requireAPIError(t *testing.T, response runtime.Object, err error, expectedErrorTypeChecker func(err error) bool, expectedErrorMessage string) { + t.Helper() + require.Nil(t, response) + require.True(t, expectedErrorTypeChecker(err)) + var status apierrors.APIStatus + errors.As(err, &status) + require.Contains(t, status.Status().Message, expectedErrorMessage) +} + +func emptyWebhookSuccessResponse() *authenticator.Response { + return &authenticator.Response{User: &user.DefaultInfo{}} +} + +func TestCreateSucceedsWhenGivenATokenAndTheWebhookAuthenticatesTheToken(t *testing.T) { + webhook := FakeToken{ + returnResponse: &authenticator.Response{ + User: &user.DefaultInfo{ + Name: "test-user", + Groups: []string{"test-group-1", "test-group-2"}, + }, + }, + returnUnauthenticated: false, + } storage := NewREST(&webhook) requestToken := "a token" - response, err := callCreate(context.Background(), storage, loginRequest(placeholderapi.LoginRequestSpec{ - Type: placeholderapi.TokenLoginCredentialType, - Token: &placeholderapi.LoginRequestTokenCredential{Value: requestToken}, - })) + + response, err := callCreate(context.Background(), storage, validLoginRequestWithToken(requestToken)) require.NoError(t, err) require.Equal(t, response, &placeholderapi.LoginRequest{ Status: placeholderapi.LoginRequestStatus{ - ExpirationTimestamp: nil, - Token: "snorlax", - ClientCertificateData: "", - ClientKeyData: "", + User: &placeholderapi.User{ + Name: "test-user", + Groups: []string{"test-group-1", "test-group-2"}, + }, + Credential: &placeholderapi.LoginRequestCredential{ + ExpirationTimestamp: nil, + Token: "snorlax", + ClientCertificateData: "", + ClientKeyData: "", + }, + Message: "", }, }) require.Equal(t, requestToken, webhook.calledWithToken) } +func TestCreateSucceedsWithAnUnauthenticatedStatusWhenGivenATokenAndTheWebhookDoesNotAuthenticateTheToken(t *testing.T) { + webhook := FakeToken{ + returnUnauthenticated: true, + } + storage := NewREST(&webhook) + requestToken := "a token" + + 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", + }, + }) + require.Equal(t, requestToken, webhook.calledWithToken) +} + +func TestCreateSucceedsWithAnUnauthenticatedStatusWhenWebhookFails(t *testing.T) { + webhook := FakeToken{ + returnErr: errors.New("some webhook error"), + } + storage := NewREST(&webhook) + + 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", + }, + }) +} + func TestCreateDoesNotPassAdditionalContextInfoToTheWebhook(t *testing.T) { - webhook := FakeToken{} + webhook := FakeToken{ + returnResponse: emptyWebhookSuccessResponse(), + } storage := NewREST(&webhook) ctx := context.WithValue(context.Background(), contextKey{}, "context-value") @@ -110,11 +180,80 @@ func TestCreateDoesNotPassAdditionalContextInfoToTheWebhook(t *testing.T) { require.Nil(t, webhook.calledWithContext.Value("context-key")) } +func TestCreateFailsWhenGivenTheWrongInputType(t *testing.T) { + notALoginRequest := runtime.Unknown{} + response, err := NewREST(&FakeToken{}).Create( + genericapirequest.NewContext(), + ¬ALoginRequest, + rest.ValidateAllObjectFunc, + &metav1.CreateOptions{}) + + requireAPIError(t, response, err, apierrors.IsBadRequest, "not a LoginRequest") +} + +func TestCreateFailsWhenTokenIsNilInRequest(t *testing.T) { + storage := NewREST(&FakeToken{}) + response, err := callCreate(context.Background(), storage, loginRequest(placeholderapi.LoginRequestSpec{ + Type: placeholderapi.TokenLoginCredentialType, + Token: nil, + })) + + requireAPIError(t, response, err, apierrors.IsInvalid, + `.placeholder.suzerain-io.github.io "request name" is invalid: spec.token.value: Required value: token must be supplied`) +} + +func TestCreateFailsWhenTypeInRequestIsMissing(t *testing.T) { + storage := NewREST(&FakeToken{}) + response, err := callCreate(context.Background(), storage, loginRequest(placeholderapi.LoginRequestSpec{ + Type: "", + Token: &placeholderapi.LoginRequestTokenCredential{Value: "a token"}, + })) + + requireAPIError(t, response, err, apierrors.IsInvalid, + `.placeholder.suzerain-io.github.io "request name" is invalid: spec.type: Required value: type must be supplied`) +} + +func TestCreateFailsWhenTypeInRequestIsNotLegal(t *testing.T) { + storage := NewREST(&FakeToken{}) + response, err := callCreate(context.Background(), storage, loginRequest(placeholderapi.LoginRequestSpec{ + Type: "this in an invalid type", + Token: &placeholderapi.LoginRequestTokenCredential{Value: "a token"}, + })) + + requireAPIError(t, response, err, apierrors.IsInvalid, + `.placeholder.suzerain-io.github.io "request name" is invalid: spec.type: Invalid value: "this in an invalid type": unrecognized type`) +} + +func TestCreateFailsWhenTokenValueIsEmptyInRequest(t *testing.T) { + storage := NewREST(&FakeToken{}) + response, err := callCreate(context.Background(), storage, loginRequest(placeholderapi.LoginRequestSpec{ + Type: placeholderapi.TokenLoginCredentialType, + Token: &placeholderapi.LoginRequestTokenCredential{Value: ""}, + })) + + requireAPIError(t, response, err, apierrors.IsInvalid, + `.placeholder.suzerain-io.github.io "request name" is invalid: spec.token.value: Required value: token must be supplied`) +} + +func TestCreateFailsWhenRequestOptionsDryRunIsNotEmpty(t *testing.T) { + response, err := NewREST(&FakeToken{}).Create( + genericapirequest.NewContext(), + validLoginRequest(), + rest.ValidateAllObjectFunc, + &metav1.CreateOptions{ + DryRun: []string{"some dry run flag"}, + }) + + requireAPIError(t, response, err, apierrors.IsInvalid, + `.placeholder.suzerain-io.github.io "request name" is invalid: dryRun: Unsupported value: []string{"some dry run flag"}`) +} + func TestCreateCancelsTheWebhookInvocationWhenTheCallToCreateIsCancelledItself(t *testing.T) { webhookStartedRunningNotificationChan := make(chan bool) webhook := FakeToken{ timeout: time.Second * 2, webhookStartedRunningNotificationChan: webhookStartedRunningNotificationChan, + returnResponse: emptyWebhookSuccessResponse(), } storage := NewREST(&webhook) ctx, cancel := context.WithCancel(context.Background()) @@ -141,6 +280,7 @@ func TestCreateAllowsTheWebhookInvocationToFinishWhenTheCallToCreateIsNotCancell webhook := FakeToken{ timeout: 0, webhookStartedRunningNotificationChan: webhookStartedRunningNotificationChan, + returnResponse: emptyWebhookSuccessResponse(), } storage := NewREST(&webhook) ctx, cancel := context.WithCancel(context.Background()) @@ -161,80 +301,3 @@ func TestCreateAllowsTheWebhookInvocationToFinishWhenTheCallToCreateIsNotCancell require.True(t, webhook.reachedTimeout) require.Equal(t, context.Canceled, webhook.calledWithContext.Err()) // the inner context is cancelled (in this case by the "defer") } - -func TestCreateFailsWhenWebhookFails(t *testing.T) { - webhook := FakeToken{ - returnErr: errors.New("some webhook error"), - } - storage := NewREST(&webhook) - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - _, err := callCreate(ctx, storage, validLoginRequest()) - require.EqualError(t, err, "authenticate token failed: some webhook error") -} - -func TestCreateFailsWhenGivenTheWrongInputType(t *testing.T) { - notALoginRequest := runtime.Unknown{} - response, err := NewREST(&FakeToken{}).Create( - genericapirequest.NewContext(), - ¬ALoginRequest, - rest.ValidateAllObjectFunc, - &metav1.CreateOptions{}) - - require.Nil(t, response) - require.True(t, apierrors.IsBadRequest(err)) - var status apierrors.APIStatus - errors.As(err, &status) - require.Contains(t, status.Status().Message, "not a LoginRequest") -} - -func TestCreateFailsWhenTokenIsNilInRequest(t *testing.T) { - storage := NewREST(&FakeToken{}) - response, err := callCreate(context.Background(), storage, loginRequest(placeholderapi.LoginRequestSpec{ - Type: placeholderapi.TokenLoginCredentialType, - Token: nil, - })) - - require.Nil(t, response) - require.True(t, apierrors.IsInvalid(err)) - var status apierrors.APIStatus - errors.As(err, &status) - require.Equal(t, - `.placeholder.suzerain-io.github.io "request name" is invalid: spec.token.value: Required value: token must be supplied`, - status.Status().Message) -} - -func TestCreateFailsWhenTokenValueIsEmptyInRequest(t *testing.T) { - storage := NewREST(&FakeToken{}) - response, err := callCreate(context.Background(), storage, loginRequest(placeholderapi.LoginRequestSpec{ - Type: placeholderapi.TokenLoginCredentialType, - Token: &placeholderapi.LoginRequestTokenCredential{Value: ""}, - })) - - require.Nil(t, response) - require.True(t, apierrors.IsInvalid(err)) - var status apierrors.APIStatus - errors.As(err, &status) - require.Equal(t, - `.placeholder.suzerain-io.github.io "request name" is invalid: spec.token.value: Required value: token must be supplied`, - status.Status().Message) -} - -func TestCreateFailsWhenRequestOptionsDryRunIsNotEmpty(t *testing.T) { - response, err := NewREST(&FakeToken{}).Create( - genericapirequest.NewContext(), - validLoginRequest(), - rest.ValidateAllObjectFunc, - &metav1.CreateOptions{ - DryRun: []string{"some dry run flag"}, - }) - - require.Nil(t, response) - require.True(t, apierrors.IsInvalid(err)) - var status apierrors.APIStatus - errors.As(err, &status) - require.Equal(t, - `.placeholder.suzerain-io.github.io "request name" is invalid: dryRun: Unsupported value: []string{"some dry run flag"}`, - status.Status().Message) -} diff --git a/test/integration/loginrequest_test.go b/test/integration/loginrequest_test.go index 9799aa14..e3b7f86b 100644 --- a/test/integration/loginrequest_test.go +++ b/test/integration/loginrequest_test.go @@ -40,20 +40,44 @@ func TestSuccessfulLoginRequest(t *testing.T) { require.NotEmptyf(t, tmcClusterToken, "must specify PLACEHOLDER_NAME_TMC_CLUSTER_TOKEN env var for integration tests") response, err := makeRequest(t, v1alpha1.LoginRequestSpec{ + Type: v1alpha1.TokenLoginCredentialType, Token: &v1alpha1.LoginRequestTokenCredential{Value: tmcClusterToken}, }) + require.NoError(t, err) + require.Empty(t, response.Status.Message) + + require.Empty(t, response.Spec) + require.NotNil(t, response.Status.Credential) + require.NotEmpty(t, response.Status.Credential.Token) + require.Empty(t, response.Status.Credential.ClientCertificateData) + require.Empty(t, response.Status.Credential.ClientKeyData) + require.Nil(t, response.Status.Credential.ExpirationTimestamp) + + require.NotNil(t, response.Status.User) + require.NotEmpty(t, response.Status.User.Name) + require.Contains(t, response.Status.User.Groups, "tmc:member") +} + +func TestFailedLoginRequestWhenTheRequestIsValidButTheTokenDoesNotAuthenticateTheUser(t *testing.T) { + response, err := makeRequest(t, v1alpha1.LoginRequestSpec{ + Type: v1alpha1.TokenLoginCredentialType, + Token: &v1alpha1.LoginRequestTokenCredential{Value: "not a good token"}, + }) + require.NoError(t, err) require.Empty(t, response.Spec) - require.NotEmpty(t, response.Status.Token) - require.Empty(t, response.Status.ClientCertificateData) - require.Empty(t, response.Status.ClientKeyData) - require.Nil(t, response.Status.ExpirationTimestamp) + require.Nil(t, response.Status.Credential) + require.Nil(t, response.Status.User) + require.Equal(t, "authentication failed", response.Status.Message) } func TestLoginRequest_ShouldFailWhenRequestDoesNotIncludeToken(t *testing.T) { - _, err := makeRequest(t, v1alpha1.LoginRequestSpec{}) + _, err := makeRequest(t, v1alpha1.LoginRequestSpec{ + Type: v1alpha1.TokenLoginCredentialType, + Token: nil, + }) require.Error(t, err) statusError, isStatus := err.(*errors.StatusError)