From 058192c22b295a58889236a4103e6079d278c417 Mon Sep 17 00:00:00 2001 From: Elio Bischof Date: Wed, 15 Feb 2023 04:21:58 +0100 Subject: [PATCH] test: fix log headers (#5222) * test: fix log headers * ensure just public types are tested * fix(postgres): proper statements for setup step 7 --------- Co-authored-by: adlerhurst --- cmd/setup/04.go | 6 +- cmd/setup/07.go | 25 ++++-- cmd/setup/07/{ => cockroach}/access.sql | 0 cmd/setup/07/{ => cockroach}/execution.sql | 0 cmd/setup/07/postgres/access.sql | 14 ++++ cmd/setup/07/postgres/execution.sql | 11 +++ cmd/setup/setup.go | 2 +- internal/logstore/emitters/access/record.go | 58 ++++++++------ .../logstore/emitters/access/record_test.go | 79 +++++++++++++++++++ 9 files changed, 158 insertions(+), 37 deletions(-) rename cmd/setup/07/{ => cockroach}/access.sql (100%) rename cmd/setup/07/{ => cockroach}/execution.sql (100%) create mode 100644 cmd/setup/07/postgres/access.sql create mode 100644 cmd/setup/07/postgres/execution.sql create mode 100644 internal/logstore/emitters/access/record_test.go diff --git a/cmd/setup/04.go b/cmd/setup/04.go index 818663e0ee..f1d512f53d 100644 --- a/cmd/setup/04.go +++ b/cmd/setup/04.go @@ -18,7 +18,7 @@ type EventstoreIndexes struct { } func (mig *EventstoreIndexes) Execute(ctx context.Context) error { - stmt, err := readStmt(mig.dbType) + stmt, err := readStmt(stmts, "04", mig.dbType, "index.sql") if err != nil { return err } @@ -30,7 +30,7 @@ func (mig *EventstoreIndexes) String() string { return "04_eventstore_indexes" } -func readStmt(typ string) (string, error) { - stmt, err := stmts.ReadFile("04/" + typ + "/index.sql") +func readStmt(fs embed.FS, folder, typ, filename string) (string, error) { + stmt, err := fs.ReadFile(folder + "/" + typ + "/" + filename) return string(stmt), err } diff --git a/cmd/setup/07.go b/cmd/setup/07.go index a1f4585c0f..f168f6d160 100644 --- a/cmd/setup/07.go +++ b/cmd/setup/07.go @@ -3,27 +3,38 @@ package setup import ( "context" "database/sql" - _ "embed" + "embed" "strings" ) var ( //go:embed 07/logstore.sql createLogstoreSchema07 string - //go:embed 07/access.sql - createAccessLogsTable07 string - //go:embed 07/execution.sql - createExecutionLogsTable07 string + //go:embed 07/cockroach/access.sql + //go:embed 07/postgres/access.sql + createAccessLogsTable07 embed.FS + //go:embed 07/cockroach/execution.sql + //go:embed 07/postgres/execution.sql + createExecutionLogsTable07 embed.FS ) type LogstoreTables struct { dbClient *sql.DB username string + dbType string } func (mig *LogstoreTables) Execute(ctx context.Context) error { - stmt := strings.ReplaceAll(createLogstoreSchema07, "%[1]s", mig.username) + createAccessLogsTable07 + createExecutionLogsTable07 - _, err := mig.dbClient.ExecContext(ctx, stmt) + accessStmt, err := readStmt(createAccessLogsTable07, "07", mig.dbType, "access.sql") + if err != nil { + return err + } + executionStmt, err := readStmt(createExecutionLogsTable07, "07", mig.dbType, "execution.sql") + if err != nil { + return err + } + stmt := strings.ReplaceAll(createLogstoreSchema07, "%[1]s", mig.username) + accessStmt + executionStmt + _, err = mig.dbClient.ExecContext(ctx, stmt) return err } diff --git a/cmd/setup/07/access.sql b/cmd/setup/07/cockroach/access.sql similarity index 100% rename from cmd/setup/07/access.sql rename to cmd/setup/07/cockroach/access.sql diff --git a/cmd/setup/07/execution.sql b/cmd/setup/07/cockroach/execution.sql similarity index 100% rename from cmd/setup/07/execution.sql rename to cmd/setup/07/cockroach/execution.sql diff --git a/cmd/setup/07/postgres/access.sql b/cmd/setup/07/postgres/access.sql new file mode 100644 index 0000000000..c440ee254d --- /dev/null +++ b/cmd/setup/07/postgres/access.sql @@ -0,0 +1,14 @@ +CREATE TABLE IF NOT EXISTS logstore.access ( + log_date TIMESTAMPTZ NOT NULL + , protocol INT NOT NULL + , request_url TEXT NOT NULL + , response_status INT NOT NULL + , request_headers JSONB + , response_headers JSONB + , instance_id TEXT NOT NULL + , project_id TEXT NOT NULL + , requested_domain TEXT + , requested_host TEXT +); + +CREATE INDEX protocol_date_desc ON logstore.access (instance_id, protocol, log_date DESC) INCLUDE (request_url, response_status, request_headers); diff --git a/cmd/setup/07/postgres/execution.sql b/cmd/setup/07/postgres/execution.sql new file mode 100644 index 0000000000..166f1895c7 --- /dev/null +++ b/cmd/setup/07/postgres/execution.sql @@ -0,0 +1,11 @@ +CREATE TABLE IF NOT EXISTS logstore.execution ( + log_date TIMESTAMPTZ NOT NULL + , took INTERVAL + , message TEXT NOT NULL + , loglevel INT NOT NULL + , instance_id TEXT NOT NULL + , action_id TEXT NOT NULL + , metadata JSONB +); + +CREATE INDEX log_date_desc ON logstore.execution (instance_id, log_date DESC) INCLUDE (took); diff --git a/cmd/setup/setup.go b/cmd/setup/setup.go index cad655081b..444c300010 100644 --- a/cmd/setup/setup.go +++ b/cmd/setup/setup.go @@ -84,7 +84,7 @@ func Setup(config *Config, steps *Steps, masterKey string) { steps.s4EventstoreIndexes = &EventstoreIndexes{dbClient: dbClient, dbType: config.Database.Type()} steps.s5LastFailed = &LastFailed{dbClient: dbClient} steps.s6OwnerRemoveColumns = &OwnerRemoveColumns{dbClient: dbClient} - steps.s7LogstoreTables = &LogstoreTables{dbClient: dbClient, username: config.Database.Username()} + steps.s7LogstoreTables = &LogstoreTables{dbClient: dbClient, username: config.Database.Username(), dbType: config.Database.Type()} err = projection.Create(ctx, dbClient, eventstoreClient, config.Projections, nil, nil) logging.OnError(err).Fatal("unable to start projections") diff --git a/internal/logstore/emitters/access/record.go b/internal/logstore/emitters/access/record.go index 17d1a75062..6b802015da 100644 --- a/internal/logstore/emitters/access/record.go +++ b/internal/logstore/emitters/access/record.go @@ -39,36 +39,41 @@ const ( func (a Record) Normalize() logstore.LogRecord { a.RequestedDomain = cutString(a.RequestedDomain, 200) a.RequestURL = cutString(a.RequestURL, 200) - normalizeHeaders(a.RequestHeaders, strings.ToLower(zitadel_http.Authorization), "grpcgateway-authorization", "cookie", "grpcgateway-cookie") - normalizeHeaders(a.ResponseHeaders, "set-cookie") + a.RequestHeaders = normalizeHeaders(a.RequestHeaders, strings.ToLower(zitadel_http.Authorization), "grpcgateway-authorization", "cookie", "grpcgateway-cookie") + a.ResponseHeaders = normalizeHeaders(a.ResponseHeaders, "set-cookie") return &a } +// normalizeHeaders lowers all header keys and redacts secrets +func normalizeHeaders(header map[string][]string, redactKeysLower ...string) map[string][]string { + return pruneKeys(redactKeys(lowerKeys(header), redactKeysLower...)) +} + +func lowerKeys(header map[string][]string) map[string][]string { + lower := make(map[string][]string, len(header)) + for k, v := range header { + lower[strings.ToLower(k)] = v + } + return lower +} + +func redactKeys(header map[string][]string, redactKeysLower ...string) map[string][]string { + redactedKeys := make(map[string][]string, len(header)) + for k, v := range header { + redactedKeys[k] = v + } + for _, redactKey := range redactKeysLower { + if _, ok := redactedKeys[redactKey]; ok { + redactedKeys[redactKey] = []string{redacted} + } + } + return redactedKeys +} + const maxValuesPerKey = 10 -// normalizeHeaders lowers all header keys and redacts secrets -func normalizeHeaders(header map[string][]string, redactKeysLower ...string) { - lowerKeys(header) - redactKeys(header, redactKeysLower...) - pruneKeys(header) -} - -func lowerKeys(header map[string][]string) { - for k, v := range header { - delete(header, k) - header[strings.ToLower(k)] = v - } -} - -func redactKeys(header map[string][]string, redactKeysLower ...string) { - for _, redactKey := range redactKeysLower { - if _, ok := header[redactKey]; ok { - header[redactKey] = []string{redacted} - } - } -} - -func pruneKeys(header map[string][]string) { +func pruneKeys(header map[string][]string) map[string][]string { + prunedKeys := make(map[string][]string, len(header)) for key, value := range header { valueItems := make([]string, 0, maxValuesPerKey) for i, valueItem := range value { @@ -79,8 +84,9 @@ func pruneKeys(header map[string][]string) { // Max 200 value length valueItems = append(valueItems, cutString(valueItem, 200)) } - header[key] = valueItems + prunedKeys[key] = valueItems } + return prunedKeys } func cutString(str string, pos int) string { diff --git a/internal/logstore/emitters/access/record_test.go b/internal/logstore/emitters/access/record_test.go new file mode 100644 index 0000000000..40cc835beb --- /dev/null +++ b/internal/logstore/emitters/access/record_test.go @@ -0,0 +1,79 @@ +package access_test + +import ( + "reflect" + "testing" + + "github.com/zitadel/zitadel/internal/logstore/emitters/access" +) + +func TestRecord_Normalize(t *testing.T) { + tests := []struct { + name string + record access.Record + want *access.Record + }{{ + name: "headers with certain keys should be redacted", + record: access.Record{ + RequestHeaders: map[string][]string{ + "authorization": {"AValue"}, + "grpcgateway-authorization": {"AValue"}, + "cookie": {"AValue"}, + "grpcgateway-cookie": {"AValue"}, + }, ResponseHeaders: map[string][]string{ + "set-cookie": {"AValue"}, + }, + }, + want: &access.Record{ + RequestHeaders: map[string][]string{ + "authorization": {"[REDACTED]"}, + "grpcgateway-authorization": {"[REDACTED]"}, + "cookie": {"[REDACTED]"}, + "grpcgateway-cookie": {"[REDACTED]"}, + }, ResponseHeaders: map[string][]string{ + "set-cookie": {"[REDACTED]"}, + }, + }, + }, { + name: "header keys should be lower cased", + record: access.Record{ + RequestHeaders: map[string][]string{"AKey": {"AValue"}}, + ResponseHeaders: map[string][]string{"AKey": {"AValue"}}}, + want: &access.Record{ + RequestHeaders: map[string][]string{"akey": {"AValue"}}, + ResponseHeaders: map[string][]string{"akey": {"AValue"}}}, + }, { + name: "an already prune record should stay unchanged", + record: access.Record{ + RequestURL: "https://my.zitadel.cloud/", + RequestHeaders: map[string][]string{ + "authorization": {"[REDACTED]"}, + }, + ResponseHeaders: map[string][]string{}, + }, + want: &access.Record{ + RequestURL: "https://my.zitadel.cloud/", + RequestHeaders: map[string][]string{ + "authorization": {"[REDACTED]"}, + }, + ResponseHeaders: map[string][]string{}, + }, + }, { + name: "empty record should stay empty", + record: access.Record{ + RequestHeaders: map[string][]string{}, + ResponseHeaders: map[string][]string{}, + }, + want: &access.Record{ + RequestHeaders: map[string][]string{}, + ResponseHeaders: map[string][]string{}, + }, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.record.Normalize(); !reflect.DeepEqual(got, tt.want) { + t.Errorf("Normalize() = %v, want %v", got, tt.want) + } + }) + } +}