From c376d06355744666026bf10182bd40bfa85cd26d Mon Sep 17 00:00:00 2001 From: Louis Ruch Date: Sat, 6 Apr 2024 13:48:47 -0700 Subject: [PATCH] feat: Add support for correlationId --- globals/globals.go | 5 + internal/credential/vault/jobs.go | 19 +++ internal/credential/vault/jobs_test.go | 41 +++++ .../credential/vault/private_credential.go | 1 + internal/credential/vault/vault.go | 6 + internal/credential/vault/vault_test.go | 30 +++- internal/daemon/controller/gateway.go | 31 ++++ internal/daemon/controller/gateway_test.go | 44 ++++++ .../handlers/targets/target_service.go | 6 + .../targets/tcp/target_service_test.go | 44 ++++++ internal/daemon/controller/interceptor.go | 38 +++++ .../daemon/controller/interceptor_test.go | 78 ++++++++++ ...add_data_key_foreign_key_references.up.sql | 3 +- ...arget_ingress_egress_worker_filters.up.sql | 2 +- .../oss/postgres/87/01_session.up.sql | 64 ++++++++ internal/errors/code.go | 2 + internal/errors/code_test.go | 5 + internal/errors/error.go | 3 + internal/errors/info.go | 4 + internal/event/context.go | 27 ++++ internal/event/context_test.go | 142 +++++++++++++++--- internal/event/event_audit.go | 41 ++--- internal/event/event_audit_test.go | 52 ++++--- internal/event/options.go | 8 + internal/event/options_test.go | 7 + internal/session/repository_session_test.go | 27 ++++ internal/session/repository_test.go | 2 + internal/session/service_list_ext_test.go | 2 +- internal/session/session.go | 10 ++ internal/session/session_test.go | 27 ++++ internal/session/testing.go | 12 ++ 31 files changed, 726 insertions(+), 57 deletions(-) create mode 100644 internal/db/schema/migrations/oss/postgres/87/01_session.up.sql diff --git a/globals/globals.go b/globals/globals.go index a98e6da8d..117d0377c 100644 --- a/globals/globals.go +++ b/globals/globals.go @@ -18,6 +18,11 @@ const ( GrantScopeThis = "this" GrantScopeChildren = "children" GrantScopeDescendants = "descendants" + + // CorrelationIdKey defines the http header and grpc metadata key used for specifying a + // correlation id. When getting the correlationId (from the http header or grpc metadata) + // ensure the comparison is case-insensitive. + CorrelationIdKey = "x-correlation-id" ) type ( diff --git a/internal/credential/vault/jobs.go b/internal/credential/vault/jobs.go index 353beb82f..b3670ab7b 100644 --- a/internal/credential/vault/jobs.go +++ b/internal/credential/vault/jobs.go @@ -549,6 +549,15 @@ func (r *CredentialRenewalJob) Run(ctx context.Context) error { if err := ctx.Err(); err != nil { return errors.Wrap(ctx, err, op) } + + if c.SessionCorrelationId != "" { + ctx, err = event.NewCorrelationIdContext(ctx, c.SessionCorrelationId) + if err != nil { + event.WriteError(ctx, op, err, event.WithInfoMsg("error generating correlation context", "credential id", c.PublicId, "session id", c.SessionId)) + continue + } + } + if err := r.renewCred(ctx, c); err != nil { event.WriteError(ctx, op, err, event.WithInfoMsg("error renewing credential", "credential id", c.PublicId)) } @@ -709,6 +718,16 @@ func (r *CredentialRevocationJob) Run(ctx context.Context) error { if err := ctx.Err(); err != nil { return errors.Wrap(ctx, err, op) } + + if c.SessionCorrelationId != "" { + ctx, err = event.NewCorrelationIdContext(ctx, c.SessionCorrelationId) + if err != nil { + // log the error, but we should still revoke the credential in Vault since it is + // no longer being used. + event.WriteError(ctx, op, err, event.WithInfoMsg("error generating correlation context", "credential id", c.PublicId, "session id", c.SessionId)) + } + } + if err := r.revokeCred(ctx, c); err != nil { event.WriteError(ctx, op, err, event.WithInfoMsg("error revoking credential", "credential id", c.PublicId)) } diff --git a/internal/credential/vault/jobs_test.go b/internal/credential/vault/jobs_test.go index 2b89339c0..c765dcdfc 100644 --- a/internal/credential/vault/jobs_test.go +++ b/internal/credential/vault/jobs_test.go @@ -22,6 +22,7 @@ import ( "github.com/hashicorp/boundary/internal/target" "github.com/hashicorp/boundary/internal/target/tcp" wrapping "github.com/hashicorp/go-kms-wrapping/v2" + "github.com/hashicorp/go-uuid" vault "github.com/hashicorp/vault/api" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -2177,6 +2178,46 @@ func TestNewCredentialCleanupJob(t *testing.T) { } } +func TestVaultJobsCorrelationId(t *testing.T) { + assert, require := assert.New(t), require.New(t) + ctx := context.Background() + conn, _ := db.TestSetup(t, "postgres") + wrapper := db.TestWrapper(t) + rw := db.New(conn) + + _, prj := iam.TestScopes(t, iam.TestRepo(t, conn, wrapper)) + cs := TestCredentialStore(t, conn, wrapper, prj.PublicId, "http://vault", "vault-token", "accessor") + lib := TestCredentialLibraries(t, conn, wrapper, cs.PublicId, 1)[0] + token := cs.Token() + + iamRepo := iam.TestRepo(t, conn, wrapper) + + // Create session with known correlationId + corId, err := uuid.GenerateUUID() + require.NoError(err) + + composedOf := session.TestSessionParams(t, conn, wrapper, iamRepo) + composedOf.CorrelationId = corId + + sess := session.TestSession(t, conn, wrapper, composedOf) + got, err := newCredential(ctx, lib.GetPublicId(), "some/vault/credential", token.GetTokenHmac(), time.Minute) + require.NoError(err) + id, err := newCredentialId(ctx) + assert.NoError(err) + got.PublicId = id + query, queryValues := insertQuery(got, sess.PublicId) + require.NoError(err) + rows, err := rw.Exec(ctx, query, queryValues) + assert.Equal(1, rows) + assert.NoError(err) + + cred := &privateCredential{PublicId: id} + err = rw.LookupById(ctx, cred) + require.NoError(err) + require.NotEmpty(cred.SessionCorrelationId) + assert.Equal(corId, cred.SessionCorrelationId) +} + func TestCredentialCleanupJob_Run(t *testing.T) { // t.Parallel() - this was causing test failures, investigate before un-commenting ctx := context.Background() diff --git a/internal/credential/vault/private_credential.go b/internal/credential/vault/private_credential.go index 86b6338e5..04ce48dea 100644 --- a/internal/credential/vault/private_credential.go +++ b/internal/credential/vault/private_credential.go @@ -48,6 +48,7 @@ type privateCredential struct { CtClientKey []byte ClientKeyHmac []byte ClientKeyId string + SessionCorrelationId string } func (pc *privateCredential) decrypt(ctx context.Context, cipher wrapping.Wrapper) error { diff --git a/internal/credential/vault/vault.go b/internal/credential/vault/vault.go index 5c8d249a7..136806b62 100644 --- a/internal/credential/vault/vault.go +++ b/internal/credential/vault/vault.go @@ -11,7 +11,9 @@ import ( "strings" "time" + "github.com/hashicorp/boundary/globals" "github.com/hashicorp/boundary/internal/errors" + "github.com/hashicorp/boundary/internal/event" "github.com/hashicorp/go-rootcerts" vault "github.com/hashicorp/vault/api" "github.com/mitchellh/mapstructure" @@ -116,6 +118,10 @@ func newClient(ctx context.Context, c *clientConfig) (*client, error) { vClient.SetNamespace(c.Namespace) } + if correlationId, ok := event.CorrelationIdFromContext(ctx); ok { + vClient.AddHeader(globals.CorrelationIdKey, correlationId) + } + return &client{ cl: vClient, token: c.Token, diff --git a/internal/credential/vault/vault_test.go b/internal/credential/vault/vault_test.go index 40fbe2ce1..231bf7c30 100644 --- a/internal/credential/vault/vault_test.go +++ b/internal/credential/vault/vault_test.go @@ -11,7 +11,10 @@ import ( "testing" "time" + "github.com/hashicorp/boundary/globals" "github.com/hashicorp/boundary/internal/errors" + "github.com/hashicorp/boundary/internal/event" + "github.com/hashicorp/go-uuid" vault "github.com/hashicorp/vault/api" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -20,19 +23,27 @@ import ( func Test_newClient(t *testing.T) { t.Parallel() v := NewTestVaultServer(t) - ctx := context.Background() + + corId, err := uuid.GenerateUUID() + require.NoError(t, err) + corCtx, err := event.NewCorrelationIdContext(context.Background(), corId) + require.NoError(t, err) tests := []struct { name string + ctx context.Context wantErr bool + wantCorId string clientConfig *clientConfig }{ { name: "nil-config", + ctx: context.Background(), wantErr: true, }, { name: "empty-addr", + ctx: context.Background(), clientConfig: &clientConfig{ Token: TokenSecret(v.RootToken), }, @@ -40,6 +51,7 @@ func Test_newClient(t *testing.T) { }, { name: "empty-token", + ctx: context.Background(), clientConfig: &clientConfig{ Addr: v.Addr, }, @@ -47,17 +59,28 @@ func Test_newClient(t *testing.T) { }, { name: "valid-config", + ctx: context.Background(), clientConfig: &clientConfig{ Addr: v.Addr, Token: TokenSecret(v.RootToken), }, + wantCorId: "", + }, + { + name: "valid-config-with-correlation-id", + ctx: corCtx, + clientConfig: &clientConfig{ + Addr: v.Addr, + Token: TokenSecret(v.RootToken), + }, + wantCorId: corId, }, } for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { assert, require := assert.New(t), require.New(t) - client, err := newClient(ctx, tt.clientConfig) + client, err := newClient(tt.ctx, tt.clientConfig) if tt.wantErr { require.Error(err) assert.Nil(client) @@ -65,6 +88,9 @@ func Test_newClient(t *testing.T) { } require.NoError(err) assert.NotNil(client) + + corIdHeader := client.cl.Headers().Get(globals.CorrelationIdKey) + assert.Equal(tt.wantCorId, corIdHeader) }) } } diff --git a/internal/daemon/controller/gateway.go b/internal/daemon/controller/gateway.go index 390f363e5..a7ad3a3eb 100644 --- a/internal/daemon/controller/gateway.go +++ b/internal/daemon/controller/gateway.go @@ -5,8 +5,11 @@ package controller import ( "context" + "fmt" "math" "net" + "net/http" + "strings" grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware" grpc_recovery "github.com/grpc-ecosystem/go-grpc-middleware/recovery" @@ -19,8 +22,10 @@ import ( "github.com/hashicorp/boundary/internal/event" "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/types/subtypes" + "github.com/hashicorp/go-uuid" "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/metadata" "google.golang.org/grpc/test/bufconn" ) @@ -52,6 +57,7 @@ func (noDelimiterStreamingMarshaler) Delimiter() []byte { func newGrpcGatewayMux() *runtime.ServeMux { return runtime.NewServeMux( + runtime.WithMetadata(correlationIdAnnotator), runtime.WithMarshalerOption(runtime.MIMEWildcard, &noDelimiterStreamingMarshaler{ &runtime.HTTPBodyMarshaler{ Marshaler: handlers.JSONMarshaler(), @@ -62,6 +68,30 @@ func newGrpcGatewayMux() *runtime.ServeMux { ) } +func correlationIdAnnotator(_ context.Context, req *http.Request) metadata.MD { + var correlationId string + for k, v := range req.Header { + if strings.ToLower(k) == globals.CorrelationIdKey { + correlationId = v[0] + break + } + } + if correlationId == "" { + var err error + correlationId, err = uuid.GenerateUUID() + + // GenerateUUID should not return an error. If it does, panic since there is no + // err return path here. + if err != nil { + panic(fmt.Sprintf("failed to generate correlation id: %v", err)) + } + } + + return metadata.New(map[string]string{ + globals.CorrelationIdKey: correlationId, + }) +} + // newGrpcServerListener will create an in-memory listener for the gRPC server. func newGrpcServerListener() grpcServerListener { buffer := globals.DefaultMaxRequestSize // seems like a reasonable size for the ring buffer, but then happily change the size if more info becomes available @@ -116,6 +146,7 @@ func newGrpcServer( grpc.UnaryInterceptor( grpc_middleware.ChainUnaryServer( unaryCtxInterceptor, // populated requestInfo from headers into the request ctx + correlationIdInterceptor(ctx), // populate correlationId from headers or generate random id errorInterceptor(ctx), // convert domain and api errors into headers for the http proxy aliasResolutionInterceptor(ctx, aliasRepoFn), // Resolve ids when an alias is provided subtypes.AttributeTransformerInterceptor(ctx), // convert to/from generic attributes from/to subtype specific attributes diff --git a/internal/daemon/controller/gateway_test.go b/internal/daemon/controller/gateway_test.go index f89b1a071..caafe7265 100644 --- a/internal/daemon/controller/gateway_test.go +++ b/internal/daemon/controller/gateway_test.go @@ -4,15 +4,19 @@ package controller import ( + "context" "fmt" "math" + "net/http" "testing" "github.com/hashicorp/boundary/api/targets" + "github.com/hashicorp/boundary/globals" _ "github.com/hashicorp/boundary/internal/daemon/controller/handlers/targets/tcp" "github.com/hashicorp/boundary/internal/iam" "github.com/hashicorp/boundary/internal/target" "github.com/hashicorp/boundary/internal/target/tcp" + "github.com/hashicorp/go-uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -48,3 +52,43 @@ func Test_gatewayDialOptions(t *testing.T) { assert.NotEmpty(res) assert.Equal(targetCount, len(res.Items)) } + +func Test_correlationIdAnnotator(t *testing.T) { + corId, err := uuid.GenerateUUID() + require.NoError(t, err) + req := &http.Request{ + Header: map[string][]string{ + globals.CorrelationIdKey: {corId}, + }, + } + + md := correlationIdAnnotator(context.Background(), req) + require.NotNil(t, md) + corIds := md.Get(globals.CorrelationIdKey) + require.Len(t, corIds, 1) + assert.Equal(t, corId, corIds[0]) + + // Now see if we do not pass a correlation id it generates one + md = correlationIdAnnotator(context.Background(), &http.Request{}) + require.NotNil(t, md) + corIds = md.Get(globals.CorrelationIdKey) + require.Len(t, corIds, 1) + assert.NotEqual(t, corId, corIds[0]) + // Validate it parses as valid uuid + _, err = uuid.ParseUUID(corIds[0]) + assert.NoError(t, err) + + // Validate correlationIdAnnotator is case-insensitive + corId, err = uuid.GenerateUUID() + require.NoError(t, err) + req = &http.Request{ + Header: map[string][]string{ + "X-CorReLAtion-id": {corId}, + }, + } + md = correlationIdAnnotator(context.Background(), req) + require.NotNil(t, md) + corIds = md.Get(globals.CorrelationIdKey) + require.Len(t, corIds, 1) + assert.Equal(t, corId, corIds[0]) +} diff --git a/internal/daemon/controller/handlers/targets/target_service.go b/internal/daemon/controller/handlers/targets/target_service.go index 833c7fc1d..dab631a37 100644 --- a/internal/daemon/controller/handlers/targets/target_service.go +++ b/internal/daemon/controller/handlers/targets/target_service.go @@ -25,6 +25,7 @@ import ( "github.com/hashicorp/boundary/internal/daemon/controller/handlers" "github.com/hashicorp/boundary/internal/db/timestamp" "github.com/hashicorp/boundary/internal/errors" + "github.com/hashicorp/boundary/internal/event" pbs "github.com/hashicorp/boundary/internal/gen/controller/api/services" intglobals "github.com/hashicorp/boundary/internal/globals" "github.com/hashicorp/boundary/internal/host" @@ -817,6 +818,10 @@ func (s Service) AuthorizeSession(ctx context.Context, req *pbs.AuthorizeSession if authResults.Error != nil { return nil, authResults.Error } + correlationId, ok := event.CorrelationIdFromContext(ctx) + if !ok { + return nil, stderrors.New("authorize session: missing correlation id") + } if authResults.RoundTripValue == nil { return nil, stderrors.New("authorize session: expected to get a target back from auth results") @@ -1039,6 +1044,7 @@ func (s Service) AuthorizeSession(ctx context.Context, req *pbs.AuthorizeSession IngressWorkerFilter: t.GetIngressWorkerFilter(), DynamicCredentials: dynCreds, StaticCredentials: staticCreds, + CorrelationId: correlationId, } if protoWorker != nil { sessionComposition.ProtocolWorkerId = protoWorker.GetPublicId() diff --git a/internal/daemon/controller/handlers/targets/tcp/target_service_test.go b/internal/daemon/controller/handlers/targets/tcp/target_service_test.go index 023be6e60..bc9db3588 100644 --- a/internal/daemon/controller/handlers/targets/tcp/target_service_test.go +++ b/internal/daemon/controller/handlers/targets/tcp/target_service_test.go @@ -34,6 +34,7 @@ import ( "github.com/hashicorp/boundary/internal/daemon/controller/handlers/credentials" "github.com/hashicorp/boundary/internal/daemon/controller/handlers/targets" "github.com/hashicorp/boundary/internal/db" + "github.com/hashicorp/boundary/internal/event" pbs "github.com/hashicorp/boundary/internal/gen/controller/api/services" authpb "github.com/hashicorp/boundary/internal/gen/controller/auth" hostplugin "github.com/hashicorp/boundary/internal/host/plugin" @@ -56,6 +57,7 @@ import ( pb "github.com/hashicorp/boundary/sdk/pbs/controller/api/resources/targets" plgpb "github.com/hashicorp/boundary/sdk/pbs/plugin" wrapping "github.com/hashicorp/go-kms-wrapping/v2" + "github.com/hashicorp/go-uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/crypto/ssh/testdata" @@ -3272,6 +3274,12 @@ func TestAuthorizeSession(t *testing.T) { require.NoError(t, err) for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { + // Generate correlation Id and add it to the context + corId, err := uuid.GenerateUUID() + require.NoError(t, err) + ctx, err = event.NewCorrelationIdContext(ctx, corId) + require.NoError(t, err) + tar := tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), tc.name, target.WithDefaultPort(defaultPort)) apiTar, err := s.AddTargetHostSources(ctx, &pbs.AddTargetHostSourcesRequest{ Id: tar.GetPublicId(), @@ -3380,6 +3388,13 @@ func TestAuthorizeSession(t *testing.T) { gotCred.Secret = nil + // CorrelationId is not part of the authSession resp, query the session directly + sessRepo, err := sessionRepoFn() + require.NoError(t, err) + gotSess, _, err := sessRepo.LookupSession(ctx, got.SessionId) + require.NoError(t, err) + require.Equal(t, corId, gotSess.CorrelationId) + got.AuthorizationToken, got.SessionId, got.CreatedTime = "", "", nil assert.Empty(t, cmp.Diff( got, @@ -3951,6 +3966,12 @@ func TestAuthorizeSessionTypedCredentials(t *testing.T) { // Tell our DB that there is a worker ready to serve the data server.TestKmsWorker(t, conn, wrapper) + // Generate correlation Id and add it to the context + corId, err := uuid.GenerateUUID() + require.NoError(t, err) + ctx, err = event.NewCorrelationIdContext(ctx, corId) + require.NoError(t, err) + asRes, err := s.AuthorizeSession(ctx, &pbs.AuthorizeSessionRequest{ Id: tar.GetPublicId(), }) @@ -4205,8 +4226,15 @@ func TestAuthorizeSession_Errors(t *testing.T) { return tr.GetItem().GetVersion() } + // Generate correlation Id and add it to the context + corId, err := uuid.GenerateUUID() + require.NoError(t, err) + ctxWithCor, err := event.NewCorrelationIdContext(ctx, corId) + require.NoError(t, err) + cases := []struct { name string + ctx context.Context setup []func(target.Target) uint32 useTargetId bool wantErr bool @@ -4215,28 +4243,33 @@ func TestAuthorizeSession_Errors(t *testing.T) { { // This one must be run first since it relies on the DB not having any worker details name: "no worker", + ctx: ctxWithCor, setup: []func(tcpTarget target.Target) uint32{hostExists, libraryExists}, useTargetId: true, wantErrContains: "No workers are available to handle this session", }, { name: "success", + ctx: ctxWithCor, setup: []func(tcpTarget target.Target) uint32{workerExists, hostExists, libraryExists}, useTargetId: true, }, { name: "no target", + ctx: ctxWithCor, setup: []func(tcpTarget target.Target) uint32{workerExists, hostExists, libraryExists}, useTargetId: false, wantErrContains: "Resource not found", }, { name: "no host port", + ctx: ctxWithCor, setup: []func(tcpTarget target.Target) uint32{workerExists, hostWithoutPort, libraryExists}, useTargetId: true, }, { name: "host port", + ctx: ctxWithCor, setup: []func(tcpTarget target.Target) uint32{ workerExists, func(tcpTarget target.Target) uint32 { tcpTarget.SetAddress("127.0.0.1:22") @@ -4252,25 +4285,36 @@ func TestAuthorizeSession_Errors(t *testing.T) { }, { name: "no hosts", + ctx: ctxWithCor, setup: []func(tcpTarget target.Target) uint32{workerExists, hostSetNoHostExists, libraryExists}, useTargetId: true, wantErrContains: "No host sources or address found for given target", }, { name: "bad library configuration", + ctx: ctxWithCor, setup: []func(tcpTarget target.Target) uint32{workerExists, hostExists, misConfiguredlibraryExists}, useTargetId: true, wantErrContains: "external system issue: error #3014: Error making API request", }, { name: "expired token library", + ctx: ctxWithCor, setup: []func(tcpTarget target.Target) uint32{workerExists, hostExists, expiredTokenLibrary}, useTargetId: true, wantErrContains: "vault.newClient: invalid configuration", }, + { + name: "no correaltion id", + ctx: ctx, + setup: []func(tcpTarget target.Target) uint32{workerExists, hostExists, libraryExists}, + useTargetId: true, + wantErrContains: "authorize session: missing correlation id", + }, } for i, tc := range cases { t.Run(tc.name, func(t *testing.T) { + ctx := tc.ctx tar := tcp.TestTarget(ctx, t, conn, proj.GetPublicId(), fmt.Sprintf("test-%d", i), target.WithDefaultPort(22)) for _, fn := range tc.setup { diff --git a/internal/daemon/controller/interceptor.go b/internal/daemon/controller/interceptor.go index 373c048f2..136aabc52 100644 --- a/internal/daemon/controller/interceptor.go +++ b/internal/daemon/controller/interceptor.go @@ -11,6 +11,7 @@ import ( "runtime/debug" grpc_recovery "github.com/grpc-ecosystem/go-grpc-middleware/recovery" + "github.com/hashicorp/boundary/globals" "github.com/hashicorp/boundary/internal/alias" commonSrv "github.com/hashicorp/boundary/internal/daemon/common" "github.com/hashicorp/boundary/internal/daemon/controller/auth" @@ -23,6 +24,7 @@ import ( pberrors "github.com/hashicorp/boundary/internal/gen/errors" "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/requests" + "github.com/hashicorp/go-uuid" "github.com/mr-tron/base58" "google.golang.org/grpc" "google.golang.org/grpc/codes" @@ -278,6 +280,42 @@ func sharedRequestInterceptorLogic( return interceptorCtx, err // not convinced we want to wrap every error and turn them into domain errors... } +func correlationIdInterceptor( + _ context.Context, +) grpc.UnaryServerInterceptor { + const op = "controller.correlationIdInterceptor" + return func(interceptorCtx context.Context, req any, + _ *grpc.UnaryServerInfo, handler grpc.UnaryHandler, + ) (any, error) { + md, ok := metadata.FromIncomingContext(interceptorCtx) + if !ok { + return nil, errors.New(interceptorCtx, errors.Internal, op, "no metadata") + } + + values := md.Get(globals.CorrelationIdKey) + if len(values) == 0 { + return nil, errors.New(interceptorCtx, errors.Internal, op, "missing correlation id metadata") + } + if len(values) > 1 { + return nil, errors.New(interceptorCtx, errors.Internal, op, fmt.Sprintf("expected 1 value for %s metadata and got %d", globals.CorrelationIdKey, len(values))) + } + correlationId := values[0] + + // Validate the correlationId + if _, err := uuid.ParseUUID(correlationId); err != nil { + return nil, errors.Wrap(interceptorCtx, err, op, errors.WithMsg("failed to validated correlation id")) + } + + interceptorCtx, err := event.NewCorrelationIdContext(interceptorCtx, correlationId) + if err != nil { + return nil, errors.Wrap(interceptorCtx, err, op, errors.WithCode(errors.Internal), errors.WithMsg("unable to create context with correlation id")) + } + + // call the handler... + return handler(interceptorCtx, req) + } +} + func errorInterceptor( _ context.Context, ) grpc.UnaryServerInterceptor { diff --git a/internal/daemon/controller/interceptor_test.go b/internal/daemon/controller/interceptor_test.go index 895c4f46b..9e0d3fb05 100644 --- a/internal/daemon/controller/interceptor_test.go +++ b/internal/daemon/controller/interceptor_test.go @@ -13,6 +13,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/hashicorp/boundary/globals" "github.com/hashicorp/boundary/internal/alias" talias "github.com/hashicorp/boundary/internal/alias/target" "github.com/hashicorp/boundary/internal/authtoken" @@ -32,6 +33,7 @@ import ( "github.com/hashicorp/boundary/internal/server" "github.com/hashicorp/boundary/internal/target/tcp" "github.com/hashicorp/go-hclog" + "github.com/hashicorp/go-uuid" "github.com/mr-tron/base58" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -988,3 +990,79 @@ func (g *testGreeter) SayHello(ctx context.Context, req *interceptor.SayHelloReq return &interceptor.SayHelloResponse{Message: "hello"}, nil } } + +func Test_correlationIdInterceptor(t *testing.T) { + interceptor := correlationIdInterceptor(context.Background()) + require.NotNil(t, interceptor) + + corId, err := uuid.GenerateUUID() + require.NoError(t, err) + + returnCtxHandler := func(ctx context.Context, req any) (any, error) { + return ctx, nil + } + + cases := []struct { + name string + ctx context.Context + wantCorId string + wantErr bool + wantErrStr string + }{ + { + name: "no metadata", + ctx: context.Background(), + wantErr: true, + wantErrStr: "controller.correlationIdInterceptor: no metadata", + }, + { + name: "no correlation id", + ctx: metadata.NewIncomingContext(context.Background(), metadata.New(map[string]string{ + "not-correlation-id": "this is not a correlation id", + })), + wantErr: true, + wantErrStr: "controller.correlationIdInterceptor: missing correlation id metadata", + }, + { + name: "too many correlation ids", + ctx: metadata.NewIncomingContext(context.Background(), metadata.New(map[string]string{ + globals.CorrelationIdKey: corId, + "x-Correlation-id": corId, // metadata.New does a toLower so this is an easy way to add multiple of same key + })), + wantErr: true, + wantErrStr: "controller.correlationIdInterceptor: expected 1 value for x-correlation-id metadata and got 2", + }, + { + name: "invalid correlation id", + ctx: metadata.NewIncomingContext(context.Background(), metadata.New(map[string]string{ + globals.CorrelationIdKey: "this is wrong", + })), + wantErr: true, + wantErrStr: "controller.correlationIdInterceptor: failed to validated correlation id", + }, + { + name: "valid correlation id", + ctx: metadata.NewIncomingContext(context.Background(), metadata.New(map[string]string{ + globals.CorrelationIdKey: corId, + })), + wantCorId: corId, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + req := &pbs.GetAccountRequest{Id: "test"} + + retCtx, err := interceptor(tc.ctx, req, nil, returnCtxHandler) + if tc.wantErr { + require.Error(t, err) + require.Contains(t, err.Error(), tc.wantErrStr) + return + } + assert.NoError(t, err) + + corId, ok := event.CorrelationIdFromContext(retCtx.(context.Context)) + require.True(t, ok) + assert.Equal(t, tc.wantCorId, corId) + }) + } +} diff --git a/internal/db/schema/migrations/oss/postgres/56/02_add_data_key_foreign_key_references.up.sql b/internal/db/schema/migrations/oss/postgres/56/02_add_data_key_foreign_key_references.up.sql index e3e8f2877..e13b66427 100644 --- a/internal/db/schema/migrations/oss/postgres/56/02_add_data_key_foreign_key_references.up.sql +++ b/internal/db/schema/migrations/oss/postgres/56/02_add_data_key_foreign_key_references.up.sql @@ -290,7 +290,8 @@ begin; 'credential_vault_token_renewal_revocation is a view where each row contains a credential store and the credential store''s data needed to connect to Vault. ' 'The view returns a separate row for each active token in Vault (current, maintaining and revoke tokens); this view should only be used for token renewal and revocation. ' 'Each row may contain encrypted data. This view should not be used to retrieve data which will be returned external to boundary.'; - + + -- Replaced in 87/01_session.up.sql create view credential_vault_credential_private as select credential.public_id as public_id, credential.library_id as library_id, diff --git a/internal/db/schema/migrations/oss/postgres/59/01_target_ingress_egress_worker_filters.up.sql b/internal/db/schema/migrations/oss/postgres/59/01_target_ingress_egress_worker_filters.up.sql index 460bbeab1..6e54fe1d8 100644 --- a/internal/db/schema/migrations/oss/postgres/59/01_target_ingress_egress_worker_filters.up.sql +++ b/internal/db/schema/migrations/oss/postgres/59/01_target_ingress_egress_worker_filters.up.sql @@ -264,7 +264,7 @@ where scd.library_id = cl.public_id and o.public_id = p.parent_id and o.type = 'org'; --- Update session immutable columns +-- Replaced in 87/01_session.up.sql drop trigger immutable_columns on session; create trigger immutable_columns before update on session for each row execute procedure immutable_columns('public_id', 'certificate', 'expiration_time', 'connection_limit', diff --git a/internal/db/schema/migrations/oss/postgres/87/01_session.up.sql b/internal/db/schema/migrations/oss/postgres/87/01_session.up.sql new file mode 100644 index 000000000..21661df75 --- /dev/null +++ b/internal/db/schema/migrations/oss/postgres/87/01_session.up.sql @@ -0,0 +1,64 @@ +-- Copyright (c) HashiCorp, Inc. +-- SPDX-License-Identifier: BUSL-1.1 + +begin; + + alter table session + add column correlation_id uuid; + + -- Replaces trigger from 59/01_target_ingress_egress_worker_filters.up.sql + drop trigger immutable_columns on session; + create trigger immutable_columns before update on session + for each row execute procedure immutable_columns('public_id', 'certificate', 'expiration_time', 'connection_limit', + 'create_time', 'endpoint', 'worker_filter', 'egress_worker_filter', 'ingress_worker_filter', 'correlation_id'); + + -- Replaces view from 56/02_add_data_key_foreign_key_references.up.sql + drop view credential_vault_credential_private; + create view credential_vault_credential_private as + select credential.public_id as public_id, + credential.library_id as library_id, + credential.session_id as session_id, + credential.create_time as create_time, + credential.update_time as update_time, + credential.version as version, + credential.external_id as external_id, + credential.last_renewal_time as last_renewal_time, + credential.expiration_time as expiration_time, + credential.is_renewable as is_renewable, + credential.status as status, + credential.last_renewal_time + (credential.expiration_time - credential.last_renewal_time) / 2 as renewal_time, + token.token_hmac as token_hmac, + token.token as ct_token, -- encrypted + token.create_time as token_create_time, + token.update_time as token_update_time, + token.last_renewal_time as token_last_renewal_time, + token.expiration_time as token_expiration_time, + token.key_id as token_key_id, + token.status as token_status, + store.project_id as project_id, + store.vault_address as vault_address, + store.namespace as namespace, + store.ca_cert as ca_cert, + store.tls_server_name as tls_server_name, + store.tls_skip_verify as tls_skip_verify, + cert.certificate as client_cert, + cert.certificate_key as ct_client_key, -- encrypted + cert.certificate_key_hmac as client_cert_key_hmac, + cert.key_id as client_key_id, + sess.correlation_id as session_correlation_id + from credential_vault_credential credential + join credential_vault_token token + on credential.token_hmac = token.token_hmac + join credential_vault_store store + on token.store_id = store.public_id + left join session sess + on credential.session_id = sess.public_id + left join credential_vault_client_certificate cert + on store.public_id = cert.store_id + where credential.expiration_time != 'infinity'::date; + comment on view credential_vault_credential_private is + 'credential_vault_credential_private is a view where each row contains a credential, ' + 'the vault token used to issue the credential, and the credential store data needed to connect to Vault. ' + 'Each row may contain encrypted data. This view should not be used to retrieve data which will be returned external to boundary.'; + +commit; diff --git a/internal/errors/code.go b/internal/errors/code.go index 6aafa83c7..839ed912c 100644 --- a/internal/errors/code.go +++ b/internal/errors/code.go @@ -128,6 +128,8 @@ const ( UnexpectedRowsAffected Code = 1107 // ImmutableColumn is used when an operation attempted to mutate an immutable column. ImmutableColumn Code = 1108 + // InvalidTextRepresentation represents a value does not have the correct text representation. + InvalidTextRepresentation Code = 1109 // Migration setup errors are codes 2000-2999 MigrationIntegrity Code = 2000 // MigrationIntegrity represents an error with the generated migration related code diff --git a/internal/errors/code_test.go b/internal/errors/code_test.go index 51b1bd80d..9d23a690a 100644 --- a/internal/errors/code_test.go +++ b/internal/errors/code_test.go @@ -420,6 +420,11 @@ func TestCode_Both_String_Info(t *testing.T) { c: InvalidListToken, want: InvalidListToken, }, + { + name: "InvalidTextRepresentation", + c: InvalidTextRepresentation, + want: InvalidTextRepresentation, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/internal/errors/error.go b/internal/errors/error.go index d81158185..9de940238 100644 --- a/internal/errors/error.go +++ b/internal/errors/error.go @@ -198,6 +198,9 @@ func Convert(e error) *Err { return E(ctx, WithoutEvent(), WithCode(ColumnNotFound), WithMsg(pgxError.Message)).(*Err) case "P0001": return E(ctx, WithoutEvent(), WithCode(Exception), WithMsg(pgxError.Message)).(*Err) + case "22P02": + return E(ctx, WithoutEvent(), WithCode(InvalidTextRepresentation), WithMsg(pgxError.Message)).(*Err) + } } // unfortunately, we can't help. diff --git a/internal/errors/info.go b/internal/errors/info.go index 8e864af5f..79ba69aac 100644 --- a/internal/errors/info.go +++ b/internal/errors/info.go @@ -43,6 +43,10 @@ var errorCodeInfo = map[Code]Info{ Message: "key/version not found", Kind: Integrity, }, + InvalidTextRepresentation: { + Message: "invalid text representation", + Kind: Integrity, + }, TicketAlreadyRedeemed: { Message: "ticket already redeemed", Kind: Integrity, diff --git a/internal/event/context.go b/internal/event/context.go index 17dc45b9c..fa20e8c53 100644 --- a/internal/event/context.go +++ b/internal/event/context.go @@ -25,6 +25,7 @@ const cancelledSendTimeout = 3 * time.Second const ( eventerKey key = iota requestInfoKey + correlationIdKey ) func init() { @@ -82,6 +83,29 @@ func RequestInfoFromContext(ctx context.Context) (*RequestInfo, bool) { return reqInfo, ok } +// NewCorrelationIdContext will return a context containing a value for the +// provided CorrelationId +func NewCorrelationIdContext(ctx context.Context, correlationId string) (context.Context, error) { + const op = "event.NewCorrelationContext" + if ctx == nil { + return nil, fmt.Errorf("%s: missing context: %w", op, ErrInvalidParameter) + } + if correlationId == "" { + return nil, fmt.Errorf("%s: missing correlation id: %w", op, ErrInvalidParameter) + } + return context.WithValue(ctx, correlationIdKey, correlationId), nil +} + +// CorrelationIdFromContext attempts to get the Correlation Id value from the context +// provided +func CorrelationIdFromContext(ctx context.Context) (string, bool) { + if ctx == nil { + return "", false + } + reqInfo, ok := ctx.Value(correlationIdKey).(string) + return reqInfo, ok +} + // WriteObservation will write an observation event. It will first check the // ctx for an eventer, then try event.SysEventer() and if no eventer can be // found an error is returned. @@ -218,6 +242,9 @@ func WriteAudit(ctx context.Context, caller Op, opt ...Option) error { return fmt.Errorf("%s: %w", op, err) } } + if correlationId, ok := CorrelationIdFromContext(ctx); ok { + opt = append(opt, withCorrelationId(correlationId)) + } e, err := newAudit(caller, opt...) if err != nil { return fmt.Errorf("%s: %w", op, err) diff --git a/internal/event/context_test.go b/internal/event/context_test.go index 8c452859d..fd49d8d03 100644 --- a/internal/event/context_test.go +++ b/internal/event/context_test.go @@ -18,6 +18,7 @@ import ( "github.com/hashicorp/eventlogger/filters/encrypt" "github.com/hashicorp/eventlogger/formatter_filters/cloudevents" "github.com/hashicorp/go-hclog" + "github.com/hashicorp/go-uuid" "github.com/mitchellh/copystructure" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -32,15 +33,16 @@ const ( ) type testAudit struct { - Id string `json:"id"` // std audit/boundary field - Version string `json:"version"` // std audit/boundary field - Type string `json:"type"` // std audit field - Timestamp time.Time `json:"timestamp"` // std audit field - RequestInfo *event.RequestInfo `json:"request_info,omitempty"` // boundary field - Auth *event.Auth `json:"auth,omitempty"` // std audit field - Request *event.Request `json:"request,omitempty"` // std audit field - Response *event.Response `json:"response,omitempty"` // std audit field - Flush bool `json:"-"` + Id string `json:"id"` // std audit/boundary field + Version string `json:"version"` // std audit/boundary field + Type string `json:"type"` // std audit field + Timestamp time.Time `json:"timestamp"` // std audit field + RequestInfo *event.RequestInfo `json:"request_info,omitempty"` // boundary field + Auth *event.Auth `json:"auth,omitempty"` // std audit field + Request *event.Request `json:"request,omitempty"` // std audit field + Response *event.Response `json:"response,omitempty"` // std audit field + Flush bool `json:"-"` + CorrelationId string `json:"correlation_id,omitempty"` } func Test_NewRequestInfoContext(t *testing.T) { @@ -670,6 +672,10 @@ func Test_WriteAudit(t *testing.T) { require.NoError(t, err) ctx, err = event.NewRequestInfoContext(ctx, info) require.NoError(t, err) + corId, err := uuid.GenerateUUID() + require.NoError(t, err) + ctx, err = event.NewCorrelationIdContext(ctx, corId) + require.NoError(t, err) testAuth := &event.Auth{ AuthTokenId: "test_auth_token_id", @@ -763,7 +769,11 @@ func Test_WriteAudit(t *testing.T) { { name: "use-syseventer", noFlush: true, - ctx: context.Background(), + ctx: func() context.Context { + ctx, err := event.NewCorrelationIdContext(context.Background(), corId) + require.NoError(t, err) + return ctx + }(), auditOpts: [][]event.Option{ { event.WithAuth( @@ -800,6 +810,7 @@ func Test_WriteAudit(t *testing.T) { } return dup.(*event.Request) }(), + CorrelationId: corId, }, setup: func() error { return event.InitSysEventer(testLogger, testLock, "use-syseventer", event.WithEventerConfig(&c.EventerConfig)) @@ -812,6 +823,8 @@ func Test_WriteAudit(t *testing.T) { noFlush: true, ctx: func() context.Context { ctx, cancel := context.WithCancel(context.Background()) + ctx, err = event.NewCorrelationIdContext(ctx, corId) + require.NoError(t, err) defer cancel() return ctx }(), @@ -839,6 +852,7 @@ func Test_WriteAudit(t *testing.T) { } return dup.(*event.Request) }(), + CorrelationId: corId, }, setup: func() error { return event.InitSysEventer(testLogger, testLock, "use-syseventer", event.WithEventerConfig(&c.EventerConfig)) @@ -859,7 +873,8 @@ func Test_WriteAudit(t *testing.T) { }, }, wantAudit: &testAudit{ - Id: "411", + Id: "411", + CorrelationId: corId, Auth: func() *event.Auth { dup, err := copystructure.Copy(testAuth) require.NoError(t, err) @@ -939,12 +954,13 @@ func Test_WriteAudit(t *testing.T) { Time: gotAudit.Time, Type: "audit", Data: map[string]any{ - "auth": tt.wantAudit.Auth, - "id": gotAudit.Data.(map[string]any)["id"], - "timestamp": now, - "request": tt.wantAudit.Request, - "type": apiRequest, - "version": testAuditVersion, + "auth": tt.wantAudit.Auth, + "id": gotAudit.Data.(map[string]any)["id"], + "timestamp": now, + "request": tt.wantAudit.Request, + "type": apiRequest, + "version": testAuditVersion, + "correlation_id": tt.wantAudit.CorrelationId, }, } if tt.wantAudit.Id != "" { @@ -1319,3 +1335,95 @@ func testLogger(t *testing.T, testLock hclog.Locker) hclog.Logger { JSONFormat: true, }) } + +func Test_NewCorrelationIdContext(t *testing.T) { + corId, err := uuid.GenerateUUID() + require.NoError(t, err) + + tests := []struct { + name string + ctx context.Context + correlationId string + wantErrIs error + wantErrContains string + }{ + { + name: "missing-ctx", + wantErrIs: event.ErrInvalidParameter, + wantErrContains: "missing context", + }, + { + name: "missing-correlation-id", + ctx: context.Background(), + wantErrIs: event.ErrInvalidParameter, + wantErrContains: "missing correlation id", + }, + { + name: "valid", + ctx: context.Background(), + correlationId: corId, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + ctx, err := event.NewCorrelationIdContext(tt.ctx, tt.correlationId) + if tt.wantErrIs != nil { + require.Errorf(err, "should have gotten an error") + assert.Nilf(ctx, "context should be nil") + assert.ErrorIs(err, tt.wantErrIs) + if tt.wantErrContains != "" { + assert.Contains(err.Error(), tt.wantErrContains) + } + return + } + require.NoError(err) + require.NotNil(ctx) + got, ok := event.CorrelationIdFromContext(ctx) + require.True(ok) + assert.Equal(tt.correlationId, got) + }) + } +} + +func Test_CorrelationIdFromContext(t *testing.T) { + corId, err := uuid.GenerateUUID() + require.NoError(t, err) + testCtx, err := event.NewCorrelationIdContext(context.Background(), corId) + require.NoError(t, err) + + tests := []struct { + name string + ctx context.Context + wantCorId string + wantNotOk bool + }{ + { + name: "missing-ctx", + wantNotOk: true, + }, + { + name: "no-correlation-id", + ctx: context.Background(), + wantNotOk: true, + }, + { + name: "valid", + ctx: testCtx, + wantCorId: corId, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + got, ok := event.CorrelationIdFromContext(tt.ctx) + if tt.wantNotOk { + require.False(ok) + assert.Empty(got) + return + } + require.True(ok) + assert.Equal(tt.wantCorId, got) + }) + } +} diff --git a/internal/event/event_audit.go b/internal/event/event_audit.go index e38a8565d..7e65d4b33 100644 --- a/internal/event/event_audit.go +++ b/internal/event/event_audit.go @@ -22,15 +22,16 @@ const ( // audit defines the data of audit events type audit struct { - Id string `json:"id"` // std audit/boundary field - Version string `json:"version"` // std audit/boundary field - Type string `json:"type"` // std audit field - Timestamp time.Time `json:"timestamp"` // std audit field - RequestInfo *RequestInfo `json:"request_info,omitempty"` // boundary field - Auth *Auth `json:"auth,omitempty"` // std audit field - Request *Request `json:"request,omitempty"` // std audit field - Response *Response `json:"response,omitempty"` // std audit field - Flush bool `json:"-"` + Id string `json:"id"` // std audit/boundary field + Version string `json:"version"` // std audit/boundary field + Type string `json:"type"` // std audit field + Timestamp time.Time `json:"timestamp"` // std audit field + RequestInfo *RequestInfo `json:"request_info,omitempty"` // boundary field + Auth *Auth `json:"auth,omitempty"` // std audit field + Request *Request `json:"request,omitempty"` // std audit field + Response *Response `json:"response,omitempty"` // std audit field + Flush bool `json:"-"` + CorrelationId string `json:"correlation_id,omitempty"` } func newAudit(fromOperation Op, opt ...Option) (*audit, error) { @@ -55,15 +56,16 @@ func newAudit(fromOperation Op, opt ...Option) (*audit, error) { } a := &audit{ - Id: opts.withId, - Version: auditVersion, - Type: string(ApiRequest), - Timestamp: dtm, - RequestInfo: opts.withRequestInfo, - Auth: opts.withAuth, - Request: opts.withRequest, - Response: opts.withResponse, - Flush: opts.withFlush, + Id: opts.withId, + Version: auditVersion, + Type: string(ApiRequest), + Timestamp: dtm, + RequestInfo: opts.withRequestInfo, + Auth: opts.withAuth, + Request: opts.withRequest, + Response: opts.withResponse, + Flush: opts.withFlush, + CorrelationId: opts.withCorrelationId, } if err := a.validate(); err != nil { return nil, fmt.Errorf("%s: %w", op, err) @@ -165,6 +167,9 @@ func (a *audit) ComposeFrom(events []*eventlogger.Event) (eventlogger.EventType, if !gated.Timestamp.IsZero() { payload.Timestamp = gated.Timestamp } + if gated.CorrelationId != "" { + payload.CorrelationId = gated.CorrelationId + } } payload.Id = validId payload.Version = auditVersion diff --git a/internal/event/event_audit_test.go b/internal/event/event_audit_test.go index 8a354f872..fe51df28d 100644 --- a/internal/event/event_audit_test.go +++ b/internal/event/event_audit_test.go @@ -8,6 +8,7 @@ import ( "time" "github.com/hashicorp/eventlogger" + "github.com/hashicorp/go-uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -16,6 +17,9 @@ func Test_newAudit(t *testing.T) { t.Parallel() testNow := time.Now() + corId, err := uuid.GenerateUUID() + require.NoError(t, err) + tests := []struct { name string fromOp Op @@ -46,17 +50,19 @@ func Test_newAudit(t *testing.T) { WithRequest(testRequest(t)), WithResponse(testResponse(t)), WithFlush(), + withCorrelationId(corId), }, want: &audit{ - Id: "all-opts", - Version: auditVersion, - Type: string(ApiRequest), - Timestamp: testNow, - RequestInfo: TestRequestInfo(t), - Auth: testAuth(t), - Request: testRequest(t), - Response: testResponse(t), - Flush: true, + Id: "all-opts", + Version: auditVersion, + Type: string(ApiRequest), + Timestamp: testNow, + RequestInfo: TestRequestInfo(t), + Auth: testAuth(t), + Request: testRequest(t), + Response: testResponse(t), + Flush: true, + CorrelationId: corId, }, }, } @@ -142,6 +148,8 @@ func TestAudit_FlushEvent(t *testing.T) { func TestAudit_ComposeFrom(t *testing.T) { t.Parallel() + corId, err := uuid.GenerateUUID() + require.NoError(t, err) testNow := time.Now() tests := []struct { name string @@ -234,6 +242,15 @@ func TestAudit_ComposeFrom(t *testing.T) { Request: testRequest(t), }, }, + { + Payload: &audit{ + Id: "valid", + Version: auditVersion, + Type: string(ApiRequest), + Timestamp: testNow, + CorrelationId: corId, + }, + }, { Payload: &audit{ Id: "valid", @@ -245,14 +262,15 @@ func TestAudit_ComposeFrom(t *testing.T) { }, }, want: audit{ - Id: "valid", - Version: auditVersion, - Type: string(ApiRequest), - Timestamp: testNow, - Auth: testAuth(t), - Request: testRequest(t), - Response: testResponse(t), - RequestInfo: TestRequestInfo(t), + Id: "valid", + Version: auditVersion, + Type: string(ApiRequest), + Timestamp: testNow, + Auth: testAuth(t), + Request: testRequest(t), + Response: testResponse(t), + RequestInfo: TestRequestInfo(t), + CorrelationId: corId, }, }, } diff --git a/internal/event/options.go b/internal/event/options.go index fed89f333..eaa80fa02 100644 --- a/internal/event/options.go +++ b/internal/event/options.go @@ -49,6 +49,7 @@ type options struct { withGating bool withNoGateLocking bool withTelemetry bool + withCorrelationId string // These options are related to the hclog adapter withHclogLevel hclog.Level @@ -132,6 +133,13 @@ func WithRequestInfo(i *RequestInfo) Option { } } +// withCorrelationId allows an optional CorrelationId +func withCorrelationId(id string) Option { + return func(o *options) { + o.withCorrelationId = id + } +} + // WithNow allows an option time.Time to represent now. func WithNow(now time.Time) Option { return func(o *options) { diff --git a/internal/event/options_test.go b/internal/event/options_test.go index 7a9916121..842512235 100644 --- a/internal/event/options_test.go +++ b/internal/event/options_test.go @@ -203,6 +203,13 @@ func Test_GetOpts(t *testing.T) { testOpts.withTelemetry = true assert.Equal(opts, testOpts) }) + t.Run("withCorrelationId", func(t *testing.T) { + assert := assert.New(t) + testOpts := getDefaultOptions() + assert.Empty(testOpts.withCorrelationId) + opts := getOpts(withCorrelationId("12345")) + assert.Equal("12345", opts.withCorrelationId) + }) } // testWrapper initializes an AEAD wrapping.Wrapper for testing. Note: this diff --git a/internal/session/repository_session_test.go b/internal/session/repository_session_test.go index be1062c4f..dd049afd0 100644 --- a/internal/session/repository_session_test.go +++ b/internal/session/repository_session_test.go @@ -32,6 +32,7 @@ import ( "github.com/hashicorp/boundary/internal/types/action" "github.com/hashicorp/boundary/internal/types/resource" wrapping "github.com/hashicorp/go-kms-wrapping/v2" + "github.com/hashicorp/go-uuid" "github.com/jackc/pgconn" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -501,6 +502,31 @@ func TestRepository_CreateSession(t *testing.T) { workerAddresses: workerAddresses, }, }, + { + name: "valid-with-correlation-id", + args: args{ + composedOf: func() ComposedOf { + c := TestSessionParams(t, conn, wrapper, iamRepo) + c.CorrelationId, err = uuid.GenerateUUID() + require.NoError(t, err) + return c + }(), + workerAddresses: workerAddresses, + }, + }, + { + name: "invalid-correlation-id", + args: args{ + composedOf: func() ComposedOf { + c := TestSessionParams(t, conn, wrapper, iamRepo) + c.CorrelationId = "invalid-format" + return c + }(), + workerAddresses: workerAddresses, + }, + wantErr: true, + wantIsError: errors.InvalidTextRepresentation, + }, { name: "invalid-protocol-worker", args: args{ @@ -642,6 +668,7 @@ func TestRepository_CreateSession(t *testing.T) { DynamicCredentials: tt.args.composedOf.DynamicCredentials, StaticCredentials: tt.args.composedOf.StaticCredentials, ProtocolWorkerId: tt.args.composedOf.ProtocolWorkerId, + CorrelationId: tt.args.composedOf.CorrelationId, } ses, err := repo.CreateSession(context.Background(), wrapper, s, tt.args.workerAddresses) diff --git a/internal/session/repository_test.go b/internal/session/repository_test.go index 9b22e45b3..1a9674631 100644 --- a/internal/session/repository_test.go +++ b/internal/session/repository_test.go @@ -158,5 +158,7 @@ func TestRepository_convertToSessions(t *testing.T) { sess.CtTofuToken = nil sess.TofuToken = nil sess.KeyId = "" + // CorrelationId is an internal attribute and not returned + sess.CorrelationId = "" assert.Equal(t, sessions[0], sess) } diff --git a/internal/session/service_list_ext_test.go b/internal/session/service_list_ext_test.go index a602e7722..539e1fc11 100644 --- a/internal/session/service_list_ext_test.go +++ b/internal/session/service_list_ext_test.go @@ -69,7 +69,7 @@ func TestService_List(t *testing.T) { require.NoError(t, err) cmpIgnoreUnexportedOpts := cmpopts.IgnoreUnexported(session.Session{}, session.State{}, timestamp.Timestamp{}, timestamppb.Timestamp{}) - cmpIgnoreFieldsOpts := cmpopts.IgnoreFields(session.Session{}, "CtCertificatePrivateKey", "CertificatePrivateKey", "KeyId") + cmpIgnoreFieldsOpts := cmpopts.IgnoreFields(session.Session{}, "CtCertificatePrivateKey", "CertificatePrivateKey", "KeyId", "CorrelationId") t.Run("List validation", func(t *testing.T) { t.Parallel() diff --git a/internal/session/session.go b/internal/session/session.go index 37f329c83..063b460ea 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -66,6 +66,8 @@ type ComposedOf struct { StaticCredentials []*StaticCredential // Which worker is performing protocol-related tasks ProtocolWorkerId string + // CorrelationId is the CorrelationId of the authorize-session request + CorrelationId string `json:"-" gorm:"default:null"` } // Session contains information about a user's session with a target @@ -138,6 +140,9 @@ type Session struct { // Connections for the session are for read only and are ignored during write operations Connections []*Connection `gorm:"-"` + // CorrelationId is the CorrelationId of the authorize-session request + CorrelationId string `json:"-" gorm:"default:null"` + tableName string `gorm:"-"` } @@ -205,6 +210,7 @@ func New(ctx context.Context, c ComposedOf, _ ...Option) (*Session, error) { DynamicCredentials: c.DynamicCredentials, StaticCredentials: c.StaticCredentials, ProtocolWorkerId: c.ProtocolWorkerId, + CorrelationId: c.CorrelationId, } if err := s.validateNewSession(ctx); err != nil { return nil, errors.Wrap(ctx, err, op) @@ -236,6 +242,7 @@ func (s *Session) Clone() any { IngressWorkerFilter: s.IngressWorkerFilter, KeyId: s.KeyId, ProtocolWorkerId: s.ProtocolWorkerId, + CorrelationId: s.CorrelationId, } if len(s.States) > 0 { clone.States = make([]*State, 0, len(s.States)) @@ -413,6 +420,9 @@ func (s *Session) validateNewSession(ctx context.Context) error { if s.CtTofuToken != nil { return errors.New(ctx, errors.InvalidParameter, op, "ct must be empty") } + if s.CorrelationId == "" { + return errors.New(ctx, errors.InvalidParameter, op, "missing correlation id") + } // It is okay for the worker filter and protocol worker ID to be empty, so // they are not checked here. return nil diff --git a/internal/session/session_test.go b/internal/session/session_test.go index 5b5d89f85..fa8839f30 100644 --- a/internal/session/session_test.go +++ b/internal/session/session_test.go @@ -18,6 +18,7 @@ import ( "github.com/hashicorp/boundary/internal/db/timestamp" "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/iam" + "github.com/hashicorp/go-uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "google.golang.org/protobuf/types/known/timestamppb" @@ -67,6 +68,7 @@ func TestSession_Create(t *testing.T) { ConnectionLimit: composedOf.ConnectionLimit, DynamicCredentials: composedOf.DynamicCredentials, StaticCredentials: composedOf.StaticCredentials, + CorrelationId: composedOf.CorrelationId, }, create: true, }, @@ -94,6 +96,7 @@ func TestSession_Create(t *testing.T) { ConnectionLimit: composedOf.ConnectionLimit, DynamicCredentials: composedOf.DynamicCredentials, StaticCredentials: composedOf.StaticCredentials, + CorrelationId: composedOf.CorrelationId, }, create: true, }, @@ -164,6 +167,19 @@ func TestSession_Create(t *testing.T) { wantErr: true, wantIsErr: errors.InvalidParameter, }, + { + name: "empty-correlationId", + args: args{ + composedOf: func() ComposedOf { + c := composedOf + c.CorrelationId = "" + return c + }(), + addresses: defaultAddresses, + }, + wantErr: true, + wantIsErr: errors.InvalidParameter, + }, { name: "empty-addresses", args: args{ @@ -184,6 +200,7 @@ func TestSession_Create(t *testing.T) { ConnectionLimit: composedOf.ConnectionLimit, DynamicCredentials: composedOf.DynamicCredentials, StaticCredentials: composedOf.StaticCredentials, + CorrelationId: composedOf.CorrelationId, }, wantAddrErr: true, wantIsErr: errors.InvalidParameter, @@ -303,6 +320,16 @@ func TestSession_Clone(t *testing.T) { cp := s.Clone() assert.Equal(cp.(*Session), s) }) + t.Run("with-correlation-id", func(t *testing.T) { + assert := assert.New(t) + composedOf := TestSessionParams(t, conn, wrapper, iamRepo) + corId, err := uuid.GenerateUUID() + require.NoError(t, err) + composedOf.CorrelationId = corId + s := TestSession(t, conn, wrapper, composedOf) + cp := s.Clone() + assert.Equal(cp.(*Session), s) + }) t.Run("not-equal", func(t *testing.T) { assert := assert.New(t) s := TestDefaultSession(t, conn, wrapper, iamRepo) diff --git a/internal/session/testing.go b/internal/session/testing.go index 542cc70ed..f9ab69385 100644 --- a/internal/session/testing.go +++ b/internal/session/testing.go @@ -22,6 +22,7 @@ import ( "github.com/hashicorp/boundary/internal/target/tcp" wrapping "github.com/hashicorp/go-kms-wrapping/v2" "github.com/hashicorp/go-secure-stdlib/base62" + "github.com/hashicorp/go-uuid" "github.com/stretchr/testify/require" "google.golang.org/protobuf/types/known/timestamppb" ) @@ -105,6 +106,11 @@ func TestSession(t testing.TB, conn *db.DB, rootWrapper wrapping.Wrapper, c Comp future := timestamppb.New(time.Now().Add(time.Hour)) c.ExpirationTime = ×tamp.Timestamp{Timestamp: future} } + if c.CorrelationId == "" { + correlationId, err := uuid.GenerateUUID() + require.NoError(err) + c.CorrelationId = correlationId + } rw := db.New(conn) s, err := New(ctx, c, opt...) require.NoError(err) @@ -191,6 +197,8 @@ func TestSessionTargetAddressParams(t testing.TB, conn *db.DB, wrapper wrapping. expTime := timestamppb.Now() expTime.Seconds += int64(tcpTarget.GetSessionMaxSeconds()) + correlationId, err := uuid.GenerateUUID() + require.NoError(err) return ComposedOf{ UserId: user.PublicId, TargetId: tcpTarget.GetPublicId(), @@ -199,6 +207,7 @@ func TestSessionTargetAddressParams(t testing.TB, conn *db.DB, wrapper wrapping. Endpoint: "tcp://127.0.0.1:22", ExpirationTime: ×tamp.Timestamp{Timestamp: expTime}, ConnectionLimit: tcpTarget.GetSessionConnectionLimit(), + CorrelationId: correlationId, } } @@ -245,6 +254,8 @@ func TestSessionParams(t testing.TB, conn *db.DB, wrapper wrapping.Wrapper, iamR expTime := timestamppb.Now() expTime.Seconds += int64(tcpTarget.GetSessionMaxSeconds()) + correlationId, err := uuid.GenerateUUID() + require.NoError(err) return ComposedOf{ UserId: user.PublicId, HostId: hosts[0].PublicId, @@ -255,6 +266,7 @@ func TestSessionParams(t testing.TB, conn *db.DB, wrapper wrapping.Wrapper, iamR Endpoint: "tcp://127.0.0.1:22", ExpirationTime: ×tamp.Timestamp{Timestamp: expTime}, ConnectionLimit: tcpTarget.GetSessionConnectionLimit(), + CorrelationId: correlationId, } }