From 18af23ca44e3cd1cf05bb42c7b2b067598a3a204 Mon Sep 17 00:00:00 2001 From: Maksym Trofimenko Date: Sun, 17 Dec 2023 23:34:21 +0000 Subject: [PATCH] add option to store ip addresses and/or user-agents in audit logs Signed-off-by: Maksym Trofimenko --- api/v2.0/swagger.yaml | 60 +++++-- .../postgresql/0140_2.11.0_schema.up.sql | 2 + src/common/const.go | 6 + .../event/handler/auditlog/auditlog.go | 18 ++- src/controller/systeminfo/controller.go | 10 ++ src/core/middlewares/middlewares.go | 2 + src/lib/config/config.go | 2 +- src/lib/config/metadata/metadatalist.go | 3 + src/lib/config/userconfig.go | 10 ++ src/lib/context.go | 32 ++++ src/pkg/audit/manager.go | 12 +- src/pkg/audit/model/model.go | 2 + .../app/base/left-side-nav/config/config.ts | 4 + .../system/system-settings.component.html | 58 +++++++ .../system/system-settings.component.ts | 2 + .../projects/projects.component.spec.ts | 8 + .../project-log/audit-log.component.html | 12 ++ .../project-log/audit-log.component.ts | 17 +- src/portal/src/app/services/app-config.ts | 2 + src/portal/src/i18n/lang/en-us-lang.json | 8 +- .../middleware/clientinfo/clientinfo.go | 85 ++++++++++ .../middleware/clientinfo/clientinfo_test.go | 151 ++++++++++++++++++ src/server/v2.0/handler/auditlog.go | 11 +- src/server/v2.0/handler/project.go | 11 +- src/server/v2.0/handler/systeminfo.go | 12 +- 25 files changed, 506 insertions(+), 34 deletions(-) create mode 100644 make/migrations/postgresql/0140_2.11.0_schema.up.sql create mode 100644 src/server/middleware/clientinfo/clientinfo.go create mode 100644 src/server/middleware/clientinfo/clientinfo_test.go diff --git a/api/v2.0/swagger.yaml b/api/v2.0/swagger.yaml index 0e4a6d503bff..7a443c124c0c 100644 --- a/api/v2.0/swagger.yaml +++ b/api/v2.0/swagger.yaml @@ -4719,7 +4719,7 @@ paths: summary: Get job log by job id description: Get job log by job id, it is only used by administrator produces: - - text/plain + - text/plain tags: - jobservice parameters: @@ -6072,7 +6072,7 @@ paths: description: Specify whether the dangerous Artifact are included inside summary information type: boolean required: false - default: false + default: false responses: '200': description: Success @@ -6091,15 +6091,15 @@ paths: get: summary: Get the vulnerability list. description: | - Get the vulnerability list. use q to pass the query condition, + Get the vulnerability list. use q to pass the query condition, supported conditions: cve_id(exact match) cvss_score_v3(range condition) severity(exact match) - repository_name(exact match) - project_id(exact match) + repository_name(exact match) + project_id(exact match) package(exact match) - tag(exact match) + tag(exact match) digest(exact match) tags: - securityhub @@ -6140,7 +6140,7 @@ paths: '401': $ref: '#/responses/401' '500': - $ref: '#/responses/500' + $ref: '#/responses/500' parameters: query: @@ -6810,6 +6810,12 @@ definitions: format: date-time example: '2006-01-02T15:04:05Z' description: The time when this operation is triggered. + client_ip: + type: string + description: Client IP address when this operation is triggered. + user_agent: + type: string + description: User agent during the operation. Metadata: type: object properties: @@ -7843,6 +7849,16 @@ definitions: x-nullable: true x-omitempty: true $ref: '#/definitions/AuthproxySetting' + audit_log_track_ip_address: + type: boolean + x-nullable: true + x-omitempty: true + description: The flag to indicate whether IP address tracking is on in audit logs. + audit_log_track_user_agent: + type: boolean + x-nullable: true + x-omitempty: true + description: The flag to indicate whether user agent tracking is on in audit logs. AuthproxySetting: type: object properties: @@ -7966,7 +7982,7 @@ definitions: type: string description: | The schedule type. The valid values are 'Hourly', 'Daily', 'Weekly', 'Custom', 'Manual', 'None' and 'Schedule'. - 'Manual' means to trigger it right away, 'Schedule' means to trigger it by a specified cron schedule and + 'Manual' means to trigger it right away, 'Schedule' means to trigger it by a specified cron schedule and 'None' means to cancel the schedule. enum: - Hourly @@ -8898,6 +8914,12 @@ definitions: skip_audit_log_database: $ref: '#/definitions/BoolConfigItem' description: Whether skip the audit log in database + audit_log_track_ip_address: + $ref: '#/definitions/BoolConfigItem' + description: Whether client ip address tracking is enabled in audit logs + audit_log_track_user_agent: + $ref: '#/definitions/BoolConfigItem' + description: Whether user agent tracking is enabled in audit logs scanner_skip_update_pulltime: $ref: '#/definitions/BoolConfigItem' description: Whether or not to skip update the pull time for scanner @@ -9178,6 +9200,16 @@ definitions: description: Skip audit log database x-omitempty: true x-isnullable: true + audit_log_track_ip_address: + type: boolean + description: Track IP addresses in audit logs + x-omitempty: true + x-isnullable: true + audit_log_track_user_agent: + type: boolean + description: Track user agent in audit logs + x-omitempty: true + x-isnullable: true session_timeout: type: integer description: The session timeout for harbor, in minutes. @@ -9772,12 +9804,12 @@ definitions: type: object description: the dangerous CVE information properties: - cve_id: + cve_id: type: string description: the cve id severity: type: string - description: the severity of the CVE + description: the severity of the CVE cvss_score_v3: type: number format: float64 @@ -9787,7 +9819,7 @@ definitions: description: the description of the CVE package: type: string - description: the package of the CVE + description: the package of the CVE version: type: string description: the version of the package @@ -9795,14 +9827,14 @@ definitions: type: object description: the dangerous artifact information properties: - project_id: + project_id: type: integer format: int64 description: the project id of the artifact repository_name: type: string description: the repository name of the artifact - digest: + digest: type: string description: the digest of the artifact critical_cnt: @@ -9862,6 +9894,6 @@ definitions: description: The description of the vulnerability links: type: array - items: + items: type: string description: Links of the vulnerability diff --git a/make/migrations/postgresql/0140_2.11.0_schema.up.sql b/make/migrations/postgresql/0140_2.11.0_schema.up.sql new file mode 100644 index 000000000000..6fc5fe881721 --- /dev/null +++ b/make/migrations/postgresql/0140_2.11.0_schema.up.sql @@ -0,0 +1,2 @@ +ALTER TABLE audit_log ADD user_agent VARCHAR(255); +ALTER TABLE audit_log ADD client_ip inet; diff --git a/src/common/const.go b/src/common/const.go index d94edf1afd96..3f9d3f4db0e8 100644 --- a/src/common/const.go +++ b/src/common/const.go @@ -211,6 +211,12 @@ const ( AuditLogForwardEndpoint = "audit_log_forward_endpoint" // SkipAuditLogDatabase skip to log audit log in database SkipAuditLogDatabase = "skip_audit_log_database" + + // AuditLogTrackIpAddress track client ip address with audit_logs + AuditLogTrackIpAddress = "audit_log_track_ip_address" + // AuditLogTrackUserAgent track user agent with audit_logs + AuditLogTrackUserAgent = "audit_log_track_user_agent" + // MaxAuditRetentionHour allowed in audit log purge MaxAuditRetentionHour = 240000 // ScannerSkipUpdatePullTime diff --git a/src/controller/event/handler/auditlog/auditlog.go b/src/controller/event/handler/auditlog/auditlog.go index 6c6b5c3e96db..4f8147dac4ab 100644 --- a/src/controller/event/handler/auditlog/auditlog.go +++ b/src/controller/event/handler/auditlog/auditlog.go @@ -16,8 +16,8 @@ package auditlog import ( "context" - "github.com/goharbor/harbor/src/controller/event" + "github.com/goharbor/harbor/src/lib" "github.com/goharbor/harbor/src/lib/config" "github.com/goharbor/harbor/src/lib/log" "github.com/goharbor/harbor/src/pkg/audit" @@ -40,7 +40,7 @@ func (h *Handler) Name() string { // Handle ... func (h *Handler) Handle(ctx context.Context, value interface{}) error { - var auditLog *am.AuditLog + var addAuditLog bool switch v := value.(type) { case *event.PushArtifactEvent, *event.DeleteArtifactEvent, @@ -60,9 +60,17 @@ func (h *Handler) Handle(ctx context.Context, value interface{}) error { log.Errorf("failed to handler event %v", err) return err } - auditLog = al - if auditLog != nil { - _, err := audit.Mgr.Create(ctx, auditLog) + + if al != nil { + ip := lib.GetClientIpAddress(ctx) + if ip != "" { + al.ClientIP = &ip + } + ua := lib.GetUserAgent(ctx) + if ua != "" { + al.UserAgent = &ua + } + _, err := audit.Mgr.Create(ctx, al) if err != nil { log.Debugf("add audit log err: %v", err) } diff --git a/src/controller/systeminfo/controller.go b/src/controller/systeminfo/controller.go index 1487f036bb70..e8fe45654b65 100644 --- a/src/controller/systeminfo/controller.go +++ b/src/controller/systeminfo/controller.go @@ -49,9 +49,15 @@ type Data struct { HarborVersion string BannerMessage string AuthProxySettings *models.HTTPAuthProxy + AuditLogs AuditLogSettings Protected *protectedData } +type AuditLogSettings struct { + TrackIPAddress bool + TrackUserAgent bool +} + type protectedData struct { CurrentTime time.Time RegistryURL string @@ -103,6 +109,10 @@ func (c *controller) GetInfo(ctx context.Context, opt Options) (*Data, error) { SelfRegistration: utils.SafeCastBool(cfg[common.SelfRegistration]), HarborVersion: fmt.Sprintf("%s-%s", version.ReleaseVersion, version.GitCommit), BannerMessage: utils.SafeCastString(mgr.Get(ctx, common.BannerMessage).GetString()), + AuditLogs: AuditLogSettings{ + TrackIPAddress: utils.SafeCastBool(cfg[common.AuditLogTrackIpAddress]), + TrackUserAgent: utils.SafeCastBool(cfg[common.AuditLogTrackUserAgent]), + }, } if res.AuthMode == common.HTTPAuth { if s, err := config.HTTPAuthProxySetting(ctx); err == nil { diff --git a/src/core/middlewares/middlewares.go b/src/core/middlewares/middlewares.go index 2d3486a0bbde..60f3c2b6ae1d 100644 --- a/src/core/middlewares/middlewares.go +++ b/src/core/middlewares/middlewares.go @@ -15,6 +15,7 @@ package middlewares import ( + "github.com/goharbor/harbor/src/server/middleware/clientinfo" "net/http" "regexp" @@ -88,6 +89,7 @@ func MiddleWares() []web.MiddleWare { session.Middleware(), csrf.Middleware(), orm.Middleware(pingSkipper), + clientinfo.Middleware(pingSkipper), notification.Middleware(pingSkipper), // notification must ahead of transaction ensure the DB transaction execution complete transaction.Middleware(dbTxSkippers...), artifactinfo.Middleware(), diff --git a/src/lib/config/config.go b/src/lib/config/config.go index 75ca6a3be820..dece323510e4 100644 --- a/src/lib/config/config.go +++ b/src/lib/config/config.go @@ -78,7 +78,7 @@ func GetManager(name string) (Manager, error) { func DefaultMgr() Manager { manager, err := GetManager(DefaultCfgManager) if err != nil { - log.Error("failed to get config manager") + log.Error("failed to get config manager", err) } return manager } diff --git a/src/lib/config/metadata/metadatalist.go b/src/lib/config/metadata/metadatalist.go index 535226bc8378..7a13fa1454a9 100644 --- a/src/lib/config/metadata/metadatalist.go +++ b/src/lib/config/metadata/metadatalist.go @@ -184,6 +184,9 @@ var ( {Name: common.AuditLogForwardEndpoint, Scope: UserScope, Group: BasicGroup, EnvKey: "AUDIT_LOG_FORWARD_ENDPOINT", DefaultValue: "", ItemType: &StringType{}, Editable: false, Description: `The endpoint to forward the audit log.`}, {Name: common.SkipAuditLogDatabase, Scope: UserScope, Group: BasicGroup, EnvKey: "SKIP_LOG_AUDIT_DATABASE", DefaultValue: "false", ItemType: &BoolType{}, Editable: false, Description: `The option to skip audit log in database`}, + {Name: common.AuditLogTrackIpAddress, Scope: UserScope, Group: BasicGroup, EnvKey: "AUDIT_LOG_TRACK_IP_ADDRESS", DefaultValue: "false", ItemType: &BoolType{}, Editable: true, Description: `The flag to enable IP addresses tracking in audit logs.`}, + {Name: common.AuditLogTrackUserAgent, Scope: UserScope, Group: BasicGroup, EnvKey: "AUDIT_LOG_TRACK_USER_AGENT", DefaultValue: "false", ItemType: &BoolType{}, Editable: true, Description: `The flag to enable user agent tracking in audit logs.`}, + {Name: common.ScannerSkipUpdatePullTime, Scope: UserScope, Group: BasicGroup, EnvKey: "SCANNER_SKIP_UPDATE_PULL_TIME", DefaultValue: "false", ItemType: &BoolType{}, Editable: false, Description: `The option to skip update pull time for scanner`}, {Name: common.SessionTimeout, Scope: UserScope, Group: BasicGroup, EnvKey: "SESSION_TIMEOUT", DefaultValue: "60", ItemType: &Int64Type{}, Editable: true, Description: `The session timeout in minutes`}, diff --git a/src/lib/config/userconfig.go b/src/lib/config/userconfig.go index e22b438f7ebd..6c7d212b39bf 100644 --- a/src/lib/config/userconfig.go +++ b/src/lib/config/userconfig.go @@ -250,6 +250,16 @@ func SkipAuditLogDatabase(ctx context.Context) bool { return DefaultMgr().Get(ctx, common.SkipAuditLogDatabase).GetBool() } +// AuditLogTrackIpAddress enables ip address tracking +func AuditLogTrackIpAddress(ctx context.Context) bool { + return DefaultMgr().Get(ctx, common.AuditLogTrackIpAddress).GetBool() +} + +// AuditLogTrackUserAgent enables user info tracking +func AuditLogTrackUserAgent(ctx context.Context) bool { + return DefaultMgr().Get(ctx, common.AuditLogTrackUserAgent).GetBool() +} + // ScannerSkipUpdatePullTime returns the scanner skip update pull time setting func ScannerSkipUpdatePullTime(ctx context.Context) bool { log.Infof("skip_update_pull_time:%v", DefaultMgr().Get(ctx, common.ScannerSkipUpdatePullTime).GetBool()) diff --git a/src/lib/context.go b/src/lib/context.go index 24ef00451c6f..3bdf129ea0cd 100644 --- a/src/lib/context.go +++ b/src/lib/context.go @@ -27,6 +27,8 @@ const ( contextKeyAuthMode contextKey = "authMode" contextKeyCarrySession contextKey = "carrySession" contextKeyRequestID contextKey = "X-Request-ID" + contextClientIpAddress contextKey = "clientIpAddress" + contextUserAgent contextKey = "userAgent" ) // ArtifactInfo wraps the artifact info extracted from the request to "/v2/" @@ -128,3 +130,33 @@ func GetXRequestID(ctx context.Context) string { } return id } + +// WithClientIpAddress returns a context with ipAddress set +func WithClientIpAddress(ctx context.Context, ipAddress string) context.Context { + return setToContext(ctx, contextClientIpAddress, ipAddress) +} + +// WithUserAgent returns a context with user agent set +func WithUserAgent(ctx context.Context, userAgent string) context.Context { + return setToContext(ctx, contextUserAgent, userAgent) +} + +// GetClientIpAddress gets the ip address from the context +func GetClientIpAddress(ctx context.Context) string { + var result string + value := getFromContext(ctx, contextClientIpAddress) + if value != nil { + result, _ = value.(string) + } + return result +} + +// GetUserAgent gets the user agent from the context +func GetUserAgent(ctx context.Context) string { + var result string + value := getFromContext(ctx, contextUserAgent) + if value != nil { + result, _ = value.(string) + } + return result +} diff --git a/src/pkg/audit/manager.go b/src/pkg/audit/manager.go index b81a89bfe094..dd3cd4a56c12 100644 --- a/src/pkg/audit/manager.go +++ b/src/pkg/audit/manager.go @@ -71,9 +71,15 @@ func (m *manager) Get(ctx context.Context, id int64) (*model.AuditLog, error) { // Create ... func (m *manager) Create(ctx context.Context, audit *model.AuditLog) (int64, error) { if len(config.AuditLogForwardEndpoint(ctx)) > 0 { - LogMgr.DefaultLogger(ctx).WithField("operator", audit.Username). - WithField("time", audit.OpTime).WithField("resourceType", audit.ResourceType). - Infof("action:%s, resource:%s", audit.Operation, audit.Resource) + logger := LogMgr.DefaultLogger(ctx).WithField("operator", audit.Username). + WithField("time", audit.OpTime).WithField("resourceType", audit.ResourceType) + if config.AuditLogTrackIpAddress(ctx) && audit.ClientIP != nil { + logger.WithField("clientIP", *audit.ClientIP) + } + if config.AuditLogTrackUserAgent(ctx) && audit.UserAgent != nil { + logger.WithField("userAgent", *audit.UserAgent) + } + logger.Infof("action:%s, resource:%s", audit.Operation, audit.Resource) } if config.SkipAuditLogDatabase(ctx) { return 0, nil diff --git a/src/pkg/audit/model/model.go b/src/pkg/audit/model/model.go index 7258473a109d..a8291c0d2924 100644 --- a/src/pkg/audit/model/model.go +++ b/src/pkg/audit/model/model.go @@ -33,6 +33,8 @@ type AuditLog struct { Resource string `orm:"column(resource)" json:"resource"` Username string `orm:"column(username)" json:"username"` OpTime time.Time `orm:"column(op_time)" json:"op_time" sort:"default:desc"` + UserAgent *string `orm:"column(user_agent)" json:"user_agent"` + ClientIP *string `orm:"column(client_ip)" json:"client_ip"` } // TableName for audit log diff --git a/src/portal/src/app/base/left-side-nav/config/config.ts b/src/portal/src/app/base/left-side-nav/config/config.ts index 0fedfca3007f..2185edd1dfb6 100644 --- a/src/portal/src/app/base/left-side-nav/config/config.ts +++ b/src/portal/src/app/base/left-side-nav/config/config.ts @@ -112,6 +112,8 @@ export class Configuration { oidc_group_filter: StringValueItem; audit_log_forward_endpoint: StringValueItem; skip_audit_log_database: BoolValueItem; + audit_log_track_ip_address: BoolValueItem; + audit_log_track_user_agent: BoolValueItem; session_timeout: NumberValueItem; scanner_skip_update_pulltime: BoolValueItem; banner_message: StringValueItem; @@ -189,6 +191,8 @@ export class Configuration { this.storage_per_project = new NumberValueItem(-1, true); this.audit_log_forward_endpoint = new StringValueItem('', true); this.skip_audit_log_database = new BoolValueItem(false, true); + this.audit_log_track_ip_address = new BoolValueItem(false, true); + this.audit_log_track_user_agent = new BoolValueItem(false, true); this.session_timeout = new NumberValueItem(60, true); this.scanner_skip_update_pulltime = new BoolValueItem(false, true); this.banner_message = new StringValueItem( diff --git a/src/portal/src/app/base/left-side-nav/config/system/system-settings.component.html b/src/portal/src/app/base/left-side-nav/config/system/system-settings.component.html index 485c31ec7114..bb0d039e8aef 100644 --- a/src/portal/src/app/base/left-side-nav/config/system/system-settings.component.html +++ b/src/portal/src/app/base/left-side-nav/config/system/system-settings.component.html @@ -346,6 +346,64 @@ " /> + + + + + + + + + + + +