From 9e88926283a7a663bfc7fd4f4aa16bd02f614176 Mon Sep 17 00:00:00 2001 From: "Alessandro (Ale) Segala" <43508+ItalyPaleAle@users.noreply.github.com> Date: Fri, 4 Apr 2025 10:05:32 -0700 Subject: [PATCH] fix: ensure indexes on audit_logs table (#415) Co-authored-by: Kyle Mendell --- .../controller/audit_log_controller.go | 6 +-- backend/internal/model/audit_log.go | 15 ++++--- backend/internal/model/oidc.go | 11 +++-- backend/internal/model/webauthn.go | 12 +++--- backend/internal/service/audit_log_service.go | 41 +++++++++++-------- ...20250403082322_audit_logs_indexes.down.sql | 3 ++ .../20250403082322_audit_logs_indexes.up.sql | 3 ++ ...20250403082322_audit_logs_indexes.down.sql | 3 ++ .../20250403082322_audit_logs_indexes.up.sql | 3 ++ 9 files changed, 63 insertions(+), 34 deletions(-) create mode 100644 backend/resources/migrations/postgres/20250403082322_audit_logs_indexes.down.sql create mode 100644 backend/resources/migrations/postgres/20250403082322_audit_logs_indexes.up.sql create mode 100644 backend/resources/migrations/sqlite/20250403082322_audit_logs_indexes.down.sql create mode 100644 backend/resources/migrations/sqlite/20250403082322_audit_logs_indexes.up.sql diff --git a/backend/internal/controller/audit_log_controller.go b/backend/internal/controller/audit_log_controller.go index 43fc8aeb..a9465a4e 100644 --- a/backend/internal/controller/audit_log_controller.go +++ b/backend/internal/controller/audit_log_controller.go @@ -102,7 +102,7 @@ func (alc *AuditLogController) listAllAuditLogsHandler(c *gin.Context) { return } - logs, pagination, err := alc.auditLogService.ListAllAuditLogs(sortedPaginationRequest, filters) + logs, pagination, err := alc.auditLogService.ListAllAuditLogs(c.Request.Context(), sortedPaginationRequest, filters) if err != nil { _ = c.Error(err) return @@ -134,7 +134,7 @@ func (alc *AuditLogController) listAllAuditLogsHandler(c *gin.Context) { // @Success 200 {array} string "List of client names" // @Router /api/audit-logs/filters/client-names [get] func (alc *AuditLogController) listClientNamesHandler(c *gin.Context) { - names, err := alc.auditLogService.ListClientNames() + names, err := alc.auditLogService.ListClientNames(c.Request.Context()) if err != nil { _ = c.Error(err) return @@ -150,7 +150,7 @@ func (alc *AuditLogController) listClientNamesHandler(c *gin.Context) { // @Success 200 {object} map[string]string "Map of user IDs to usernames" // @Router /api/audit-logs/filters/users [get] func (alc *AuditLogController) listUserNamesWithIdsHandler(c *gin.Context) { - users, err := alc.auditLogService.ListUsernamesWithIds() + users, err := alc.auditLogService.ListUsernamesWithIds(c.Request.Context()) if err != nil { _ = c.Error(err) return diff --git a/backend/internal/model/audit_log.go b/backend/internal/model/audit_log.go index 63ea3ebc..813242ab 100644 --- a/backend/internal/model/audit_log.go +++ b/backend/internal/model/audit_log.go @@ -3,7 +3,7 @@ package model import ( "database/sql/driver" "encoding/json" - "errors" + "fmt" ) type AuditLog struct { @@ -34,7 +34,7 @@ const ( // Scan and Value methods for GORM to handle the custom type -func (e *AuditLogEvent) Scan(value interface{}) error { +func (e *AuditLogEvent) Scan(value any) error { *e = AuditLogEvent(value.(string)) return nil } @@ -43,11 +43,14 @@ func (e AuditLogEvent) Value() (driver.Value, error) { return string(e), nil } -func (d *AuditLogData) Scan(value interface{}) error { - if v, ok := value.([]byte); ok { +func (d *AuditLogData) Scan(value any) error { + switch v := value.(type) { + case []byte: return json.Unmarshal(v, d) - } else { - return errors.New("type assertion to []byte failed") + case string: + return json.Unmarshal([]byte(v), d) + default: + return fmt.Errorf("unsupported type: %T", value) } } diff --git a/backend/internal/model/oidc.go b/backend/internal/model/oidc.go index 7a138b1a..725cec92 100644 --- a/backend/internal/model/oidc.go +++ b/backend/internal/model/oidc.go @@ -3,7 +3,7 @@ package model import ( "database/sql/driver" "encoding/json" - "errors" + "fmt" datatype "github.com/pocket-id/pocket-id/backend/internal/model/types" "gorm.io/gorm" @@ -74,10 +74,13 @@ func (c *OidcClient) AfterFind(_ *gorm.DB) (err error) { type UrlList []string //nolint:recvcheck func (cu *UrlList) Scan(value interface{}) error { - if v, ok := value.([]byte); ok { + switch v := value.(type) { + case []byte: return json.Unmarshal(v, cu) - } else { - return errors.New("type assertion to []byte failed") + case string: + return json.Unmarshal([]byte(v), cu) + default: + return fmt.Errorf("unsupported type: %T", value) } } diff --git a/backend/internal/model/webauthn.go b/backend/internal/model/webauthn.go index 1991e40d..bbe2ddf3 100644 --- a/backend/internal/model/webauthn.go +++ b/backend/internal/model/webauthn.go @@ -3,7 +3,7 @@ package model import ( "database/sql/driver" "encoding/json" - "errors" + "fmt" "time" "github.com/go-webauthn/webauthn/protocol" @@ -49,11 +49,13 @@ type AuthenticatorTransportList []protocol.AuthenticatorTransport //nolint:recvc // Scan and Value methods for GORM to handle the custom type func (atl *AuthenticatorTransportList) Scan(value interface{}) error { - - if v, ok := value.([]byte); ok { + switch v := value.(type) { + case []byte: return json.Unmarshal(v, atl) - } else { - return errors.New("type assertion to []byte failed") + case string: + return json.Unmarshal([]byte(v), atl) + default: + return fmt.Errorf("unsupported type: %T", value) } } diff --git a/backend/internal/service/audit_log_service.go b/backend/internal/service/audit_log_service.go index 678dfa23..aaf4c8b3 100644 --- a/backend/internal/service/audit_log_service.go +++ b/backend/internal/service/audit_log_service.go @@ -1,6 +1,7 @@ package service import ( + "context" "fmt" "log" @@ -100,10 +101,13 @@ func (s *AuditLogService) DeviceStringFromUserAgent(userAgent string) string { return ua.Name + " on " + ua.OS + " " + ua.OSVersion } -func (s *AuditLogService) ListAllAuditLogs(sortedPaginationRequest utils.SortedPaginationRequest, filters dto.AuditLogFilterDto) ([]model.AuditLog, utils.PaginationResponse, error) { +func (s *AuditLogService) ListAllAuditLogs(ctx context.Context, sortedPaginationRequest utils.SortedPaginationRequest, filters dto.AuditLogFilterDto) ([]model.AuditLog, utils.PaginationResponse, error) { var logs []model.AuditLog - query := s.db.Preload("User").Model(&model.AuditLog{}) + query := s.db. + WithContext(ctx). + Preload("User"). + Model(&model.AuditLog{}) if filters.UserID != "" { query = query.Where("user_id = ?", filters.UserID) @@ -131,8 +135,11 @@ func (s *AuditLogService) ListAllAuditLogs(sortedPaginationRequest utils.SortedP return logs, pagination, nil } -func (s *AuditLogService) ListUsernamesWithIds() (users map[string]string, err error) { - query := s.db.Joins("User").Model(&model.AuditLog{}). +func (s *AuditLogService) ListUsernamesWithIds(ctx context.Context) (users map[string]string, err error) { + query := s.db. + WithContext(ctx). + Joins("User"). + Model(&model.AuditLog{}). Select("DISTINCT User.id, User.username"). Where("User.username IS NOT NULL") @@ -146,7 +153,7 @@ func (s *AuditLogService) ListUsernamesWithIds() (users map[string]string, err e return nil, fmt.Errorf("failed to query user IDs: %w", err) } - users = make(map[string]string) + users = make(map[string]string, len(results)) for _, result := range results { users[result.ID] = result.Username } @@ -154,25 +161,27 @@ func (s *AuditLogService) ListUsernamesWithIds() (users map[string]string, err e return users, nil } -func (s *AuditLogService) ListClientNames() (clientNames []string, err error) { - dialect := s.db.Name() - var query *gorm.DB +func (s *AuditLogService) ListClientNames(ctx context.Context) (clientNames []string, err error) { + query := s.db. + WithContext(ctx). + Model(&model.AuditLog{}) + dialect := s.db.Name() switch dialect { case "sqlite": - query = s.db.Model(&model.AuditLog{}). - Select("DISTINCT json_extract(data, '$.clientName') as clientName"). + query = query. + Select("DISTINCT json_extract(data, '$.clientName') as client_name"). Where("json_extract(data, '$.clientName') IS NOT NULL") case "postgres": - query = s.db.Model(&model.AuditLog{}). - Select("DISTINCT data->>'clientName' as clientName"). + query = query. + Select("DISTINCT data->>'clientName' as client_name"). Where("data->>'clientName' IS NOT NULL") default: return nil, fmt.Errorf("unsupported database dialect: %s", dialect) } type Result struct { - ClientName string `gorm:"column:clientName"` + ClientName string `gorm:"column:client_name"` } var results []Result @@ -180,9 +189,9 @@ func (s *AuditLogService) ListClientNames() (clientNames []string, err error) { return nil, fmt.Errorf("failed to query client IDs: %w", err) } - for _, result := range results { - clientNames = append(clientNames, result.ClientName) - + clientNames = make([]string, len(results)) + for i, result := range results { + clientNames[i] = result.ClientName } return clientNames, nil diff --git a/backend/resources/migrations/postgres/20250403082322_audit_logs_indexes.down.sql b/backend/resources/migrations/postgres/20250403082322_audit_logs_indexes.down.sql new file mode 100644 index 00000000..fa8058b6 --- /dev/null +++ b/backend/resources/migrations/postgres/20250403082322_audit_logs_indexes.down.sql @@ -0,0 +1,3 @@ +DROP INDEX IF EXISTS idx_audit_logs_event; +DROP INDEX IF EXISTS idx_audit_logs_user_id; +DROP INDEX IF EXISTS idx_audit_logs_client_name; diff --git a/backend/resources/migrations/postgres/20250403082322_audit_logs_indexes.up.sql b/backend/resources/migrations/postgres/20250403082322_audit_logs_indexes.up.sql new file mode 100644 index 00000000..71158a33 --- /dev/null +++ b/backend/resources/migrations/postgres/20250403082322_audit_logs_indexes.up.sql @@ -0,0 +1,3 @@ +CREATE INDEX idx_audit_logs_event ON audit_logs(event); +CREATE INDEX idx_audit_logs_user_id ON audit_logs(user_id); +CREATE INDEX idx_audit_logs_client_name ON audit_logs(("data"->>'clientName')); diff --git a/backend/resources/migrations/sqlite/20250403082322_audit_logs_indexes.down.sql b/backend/resources/migrations/sqlite/20250403082322_audit_logs_indexes.down.sql new file mode 100644 index 00000000..fa8058b6 --- /dev/null +++ b/backend/resources/migrations/sqlite/20250403082322_audit_logs_indexes.down.sql @@ -0,0 +1,3 @@ +DROP INDEX IF EXISTS idx_audit_logs_event; +DROP INDEX IF EXISTS idx_audit_logs_user_id; +DROP INDEX IF EXISTS idx_audit_logs_client_name; diff --git a/backend/resources/migrations/sqlite/20250403082322_audit_logs_indexes.up.sql b/backend/resources/migrations/sqlite/20250403082322_audit_logs_indexes.up.sql new file mode 100644 index 00000000..a9011ba2 --- /dev/null +++ b/backend/resources/migrations/sqlite/20250403082322_audit_logs_indexes.up.sql @@ -0,0 +1,3 @@ +CREATE INDEX idx_audit_logs_event ON audit_logs(event); +CREATE INDEX idx_audit_logs_user_id ON audit_logs(user_id); +CREATE INDEX idx_audit_logs_client_name ON audit_logs((json_extract(data, '$.clientName')));