Skip to content

Commit

Permalink
add option to store ip addresses and/or user-agents in audit logs
Browse files Browse the repository at this point in the history
fix ui tests

Signed-off-by: Maksym Trofimenko <maksym@container-registry.com>
  • Loading branch information
Maksym Trofimenko committed Mar 24, 2024
1 parent 69fc957 commit 813eb07
Show file tree
Hide file tree
Showing 27 changed files with 503 additions and 22 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ misspell:
@echo checking misspell...
@find . -type d \( -path ./tests \) -prune -o -name '*.go' -print | xargs misspell -error

# golangci-lint binary installation or refer to https://golangci-lint.run/usage/install/#local-installation
# golangci-lint binary installation or refer to https://golangci-lint.run/usage/install/#local-installation
# curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.55.2
GOLANGCI_LINT := $(shell go env GOPATH)/bin/golangci-lint
lint:
Expand Down
32 changes: 32 additions & 0 deletions api/v2.0/swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6833,6 +6833,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:
Expand Down Expand Up @@ -7871,6 +7877,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.
oidc_provider_name:
type: string
x-nullable: true
Expand Down Expand Up @@ -8931,6 +8947,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
Expand Down Expand Up @@ -9211,6 +9233,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.
Expand Down
5 changes: 4 additions & 1 deletion make/migrations/postgresql/0140_2.11.0_schema.up.sql
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
ALTER TABLE audit_log ADD user_agent VARCHAR(255);
ALTER TABLE audit_log ADD client_ip inet;

/*
table artifact:
id SERIAL PRIMARY KEY NOT NULL,
Expand Down Expand Up @@ -28,4 +31,4 @@ then set column artifact_type as not null
*/
UPDATE artifact SET artifact_type = media_type;

ALTER TABLE artifact ALTER COLUMN artifact_type SET NOT NULL;
ALTER TABLE artifact ALTER COLUMN artifact_type SET NOT NULL;
6 changes: 6 additions & 0 deletions src/common/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,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
Expand Down
16 changes: 12 additions & 4 deletions src/controller/event/handler/auditlog/auditlog.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ 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"
Expand All @@ -40,7 +41,6 @@ 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,
Expand All @@ -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)
}
Expand Down
10 changes: 10 additions & 0 deletions src/controller/systeminfo/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,16 @@ type Data struct {
HarborVersion string
BannerMessage string
AuthProxySettings *models.HTTPAuthProxy
AuditLogs AuditLogSettings
Protected *protectedData
OIDCProviderName string
}

type AuditLogSettings struct {
TrackIPAddress bool
TrackUserAgent bool
}

type protectedData struct {
CurrentTime time.Time
RegistryURL string
Expand Down Expand Up @@ -105,6 +111,10 @@ func (c *controller) GetInfo(ctx context.Context, opt Options) (*Data, error) {
HarborVersion: fmt.Sprintf("%s-%s", version.ReleaseVersion, version.GitCommit),
BannerMessage: utils.SafeCastString(mgr.Get(ctx, common.BannerMessage).GetString()),
OIDCProviderName: OIDCProviderName(cfg),
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 {
Expand Down
3 changes: 3 additions & 0 deletions src/core/middlewares/middlewares.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import (
"net/http"
"regexp"

"github.com/goharbor/harbor/src/server/middleware/clientinfo"

"github.com/beego/beego/v2/server/web"

"github.com/goharbor/harbor/src/pkg/distribution"
Expand Down Expand Up @@ -94,6 +96,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(),
Expand Down
2 changes: 1 addition & 1 deletion src/lib/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
3 changes: 3 additions & 0 deletions src/lib/config/metadata/metadatalist.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,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`},
Expand Down
10 changes: 10 additions & 0 deletions src/lib/config/userconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,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 {
return DefaultMgr().Get(ctx, common.ScannerSkipUpdatePullTime).GetBool()
Expand Down
32 changes: 32 additions & 0 deletions src/lib/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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/"
Expand Down Expand Up @@ -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
}
12 changes: 9 additions & 3 deletions src/pkg/audit/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,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
Expand Down
2 changes: 2 additions & 0 deletions src/pkg/audit/model/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions src/portal/src/app/base/left-side-nav/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,64 @@
" />
</clr-checkbox-wrapper>
</clr-checkbox-container>
<clr-checkbox-container class="center">
<label for="auditLogTrackIpAddress"
>{{ 'AUDIT_LOG.TRACK_IP' | translate }}
<clr-tooltip>
<clr-icon
clrTooltipTrigger
shape="info-circle"
size="24"></clr-icon>
<clr-tooltip-content
clrPosition="top-right"
clrSize="lg"
*clrIfOpen>
<span>{{
'AUDIT_LOG.TRACK_IP_TOOLTIP' | translate
}}</span>
</clr-tooltip-content>
</clr-tooltip>
</label>
<clr-checkbox-wrapper>
<input
type="checkbox"
clrCheckbox
name="auditLogTrackIpAddress"
id="auditLogTrackIpAddress"
[(ngModel)]="
currentConfig.audit_log_track_ip_address.value
" />
</clr-checkbox-wrapper>
</clr-checkbox-container>
<clr-checkbox-container class="center">
<label for="auditLogTrackUserAgent"
>{{ 'AUDIT_LOG.TRACK_UA' | translate }}
<clr-tooltip>
<clr-icon
clrTooltipTrigger
shape="info-circle"
size="24"></clr-icon>
<clr-tooltip-content
clrPosition="top-right"
clrSize="lg"
*clrIfOpen>
<span>{{
'AUDIT_LOG.TRACK_UA_TOOLTIP' | translate
}}</span>
</clr-tooltip-content>
</clr-tooltip>
</label>
<clr-checkbox-wrapper>
<input
type="checkbox"
clrCheckbox
name="auditLogTrackUserAgent"
id="auditLogTrackUserAgent"
[(ngModel)]="
currentConfig.audit_log_track_user_agent.value
" />
</clr-checkbox-wrapper>
</clr-checkbox-container>
<div class="clr-form-control">
<label class="clr-control-label">{{
'BANNER_MESSAGE.BANNER_MESSAGE' | translate
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,8 @@ export class SystemSettingsComponent implements OnInit, OnDestroy {
prop === 'robot_name_prefix' ||
prop === 'audit_log_forward_endpoint' ||
prop === 'skip_audit_log_database' ||
prop === 'audit_log_track_ip_address' ||
prop === 'audit_log_track_user_agent' ||
prop === 'session_timeout' ||
prop === 'scanner_skip_update_pulltime' ||
prop === 'banner_message'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,14 @@ describe('ProjectComponent', () => {
value: false,
editable: true,
},
audit_log_track_ip_address: {
value: false,
editable: true,
},
audit_log_track_user_agent: {
value: false,
editable: true,
},
});
},
};
Expand Down
Loading

0 comments on commit 813eb07

Please sign in to comment.