From 1af803780cd6bee067ac67dd0d8ba410a3d360f9 Mon Sep 17 00:00:00 2001 From: Timothy Messier Date: Wed, 6 Apr 2022 09:05:22 -0400 Subject: [PATCH] feat(controller): Reorder interceptor stack The new `subtype.AttributeTransformerInterceptor` can return an error if it fails to transform the attributes. This is mainly if invalid attribute fields were included for a particular subtype. This error case used to be checked in the grpc service handler functions. However, now that it is happening in the interceptor, the `errorInterceptor` needs to be moved up in the stack so that it will process the response of the new transformation interceptor. The `errorInterceptor` used to be after the `auditRequestInterceptor` and prior to the `auditResponseInterceptor`, meaning that the `auditResponseInterceptor` would have access to the result of the `errorInterceptor`. But it seems the `auditRequestInterceptor` does not use or need any of the headers set by the `errorInterceptor` so it should be safe to move the `errorInterceptor`. This is important since the `subtype.AttributeTransformerInterceptor` needs to be before the audit interceptors in the stack to properly transform requests/responses so they can be audited with the complete classification information. --- internal/servers/controller/gateway.go | 2 +- internal/tests/api/accounts/account_test.go | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/internal/servers/controller/gateway.go b/internal/servers/controller/gateway.go index 397486684..05243cb07 100644 --- a/internal/servers/controller/gateway.go +++ b/internal/servers/controller/gateway.go @@ -75,9 +75,9 @@ func newGrpcServer( grpc.UnaryInterceptor( grpc_middleware.ChainUnaryServer( requestCtxInterceptor, // populated requestInfo from headers into the request ctx + errorInterceptor(ctx), // convert domain and api errors into headers for the http proxy subtypes.AttributeTransformerInterceptor(ctx), // convert to/from generic attributes from/to subtype specific attributes auditRequestInterceptor(ctx), // before we get started, audit the request - errorInterceptor(ctx), // convert domain and api errors into headers for the http proxy statusCodeInterceptor(ctx), // convert grpc codes into http status codes for the http proxy (can modify the resp) auditResponseInterceptor(ctx), // as we finish, audit the response grpc_recovery.UnaryServerInterceptor( // recover from panics with a grpc internal error diff --git a/internal/tests/api/accounts/account_test.go b/internal/tests/api/accounts/account_test.go index 671d0af8c..1c4f9197a 100644 --- a/internal/tests/api/accounts/account_test.go +++ b/internal/tests/api/accounts/account_test.go @@ -382,4 +382,12 @@ func TestErrorsOidc(t *testing.T) { apiErr = api.AsServerError(err) require.NotNil(apiErr) assert.EqualValues(http.StatusBadRequest, apiErr.Response().StatusCode()) + + // Invalid attribute fields + _, err = accountClient.Create(tc.Context(), amId, + accounts.WithOidcAccountSubject("subject2"), + accounts.WithPasswordAccountLoginName("foo"), + ) + require.Error(err) + require.JSONEq(err.Error(), `{"kind":"InvalidArgument", "message":"Error in provided request.", "details":{"request_fields":[{"name":"attributes", "description":"Attribute fields do not match the expected format."}]}}`) }