From f44c89d200544514882706951e24872147b7c6b1 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 26 Feb 2024 08:27:33 -0600 Subject: [PATCH] chore: enforce orgid in audit logs where required (#12283) * chore: enforce orgid in audit logs where required --- coderd/audit/request.go | 62 +++++++++++++++++++++++++++++--- coderd/templates.go | 36 ++++++++++--------- coderd/templateversions.go | 36 ++++++++++--------- coderd/workspaces.go | 64 +++++++++++++++++++--------------- enterprise/coderd/groups.go | 27 +++++++------- enterprise/coderd/templates.go | 9 ++--- 6 files changed, 154 insertions(+), 80 deletions(-) diff --git a/coderd/audit/request.go b/coderd/audit/request.go index c61db9ef83..130ce9dd56 100644 --- a/coderd/audit/request.go +++ b/coderd/audit/request.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "encoding/json" + "flag" "fmt" "net" "net/http" @@ -25,6 +26,9 @@ type RequestParams struct { Audit Auditor Log slog.Logger + // OrganizationID is only provided when possible. If an audit resource extends + // beyond the org scope, leave this as the nil uuid. + OrganizationID uuid.UUID Request *http.Request Action database.AuditAction AdditionalFields json.RawMessage @@ -96,7 +100,7 @@ func ResourceTarget[T Auditable](tgt T) string { case database.HealthSettings: return "" // no target? default: - panic(fmt.Sprintf("unknown resource %T", tgt)) + panic(fmt.Sprintf("unknown resource %T for ResourceTarget", tgt)) } } @@ -129,7 +133,7 @@ func ResourceID[T Auditable](tgt T) uuid.UUID { // Artificial ID for auditing purposes return typed.ID default: - panic(fmt.Sprintf("unknown resource %T", tgt)) + panic(fmt.Sprintf("unknown resource %T for ResourceID", tgt)) } } @@ -160,10 +164,59 @@ func ResourceType[T Auditable](tgt T) database.ResourceType { case database.HealthSettings: return database.ResourceTypeHealthSettings default: - panic(fmt.Sprintf("unknown resource %T", typed)) + panic(fmt.Sprintf("unknown resource %T for ResourceType", typed)) } } +// ResourceRequiresOrgID will ensure given resources are always audited with an +// organization ID. +func ResourceRequiresOrgID[T Auditable]() bool { + var tgt T + switch any(tgt).(type) { + case database.Template, database.TemplateVersion: + return true + case database.Workspace, database.WorkspaceBuild: + return true + case database.AuditableGroup: + return true + case database.User: + return false + case database.GitSSHKey: + return false + case database.APIKey: + return false + case database.License: + return false + case database.WorkspaceProxy: + return false + case database.AuditOAuthConvertState: + // The merge state is for the given user + return false + case database.HealthSettings: + // Artificial ID for auditing purposes + return false + default: + panic(fmt.Sprintf("unknown resource %T for ResourceRequiresOrgID", tgt)) + } +} + +// requireOrgID will either panic (in unit tests) or log an error (in production) +// if the given resource requires an organization ID and the provided ID is nil. +func requireOrgID[T Auditable](ctx context.Context, id uuid.UUID, log slog.Logger) uuid.UUID { + if ResourceRequiresOrgID[T]() && id == uuid.Nil { + var tgt T + resourceName := fmt.Sprintf("%T", tgt) + if flag.Lookup("test.v") != nil { + // In unit tests we panic to fail the tests + panic(fmt.Sprintf("missing required organization ID for resource %q", resourceName)) + } + log.Error(ctx, "missing required organization ID for resource in audit log", + slog.F("resource", resourceName), + ) + } + return id +} + // InitRequest initializes an audit log for a request. It returns a function // that should be deferred, causing the audit log to be committed when the // handler returns. @@ -243,6 +296,7 @@ func InitRequest[T Auditable](w http.ResponseWriter, p *RequestParams) (*Request StatusCode: int32(sw.Status), RequestID: httpmw.RequestID(p.Request), AdditionalFields: p.AdditionalFields, + OrganizationID: requireOrgID[T](logCtx, p.OrganizationID, p.Log), } err := p.Audit.Export(ctx, auditLog) if err != nil { @@ -276,7 +330,7 @@ func BackgroundAudit[T Auditable](ctx context.Context, p *BackgroundAuditParams[ ID: uuid.New(), Time: dbtime.Now(), UserID: p.UserID, - OrganizationID: p.OrganizationID, + OrganizationID: requireOrgID[T](ctx, p.OrganizationID, p.Log), Ip: ip, UserAgent: sql.NullString{}, ResourceType: either(p.Old, p.New, ResourceType[T], p.Action), diff --git a/coderd/templates.go b/coderd/templates.go index 7b23c910fa..d7c578bd63 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -57,10 +57,11 @@ func (api *API) deleteTemplate(rw http.ResponseWriter, r *http.Request) { template = httpmw.TemplateParam(r) auditor = *api.Auditor.Load() aReq, commitAudit = audit.InitRequest[database.Template](rw, &audit.RequestParams{ - Audit: auditor, - Log: api.Logger, - Request: r, - Action: database.AuditActionDelete, + Audit: auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionDelete, + OrganizationID: template.OrganizationID, }) ) defer commitAudit() @@ -123,16 +124,18 @@ func (api *API) postTemplateByOrganization(rw http.ResponseWriter, r *http.Reque apiKey = httpmw.APIKey(r) auditor = *api.Auditor.Load() templateAudit, commitTemplateAudit = audit.InitRequest[database.Template](rw, &audit.RequestParams{ - Audit: auditor, - Log: api.Logger, - Request: r, - Action: database.AuditActionCreate, + Audit: auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionCreate, + OrganizationID: organization.ID, }) templateVersionAudit, commitTemplateVersionAudit = audit.InitRequest[database.TemplateVersion](rw, &audit.RequestParams{ - Audit: auditor, - Log: api.Logger, - Request: r, - Action: database.AuditActionWrite, + Audit: auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionWrite, + OrganizationID: organization.ID, }) ) defer commitTemplateAudit() @@ -542,10 +545,11 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) { auditor = *api.Auditor.Load() portSharer = *api.PortSharer.Load() aReq, commitAudit = audit.InitRequest[database.Template](rw, &audit.RequestParams{ - Audit: auditor, - Log: api.Logger, - Request: r, - Action: database.AuditActionWrite, + Audit: auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionWrite, + OrganizationID: template.OrganizationID, }) ) defer commitAudit() diff --git a/coderd/templateversions.go b/coderd/templateversions.go index 627e72f023..ca8f660454 100644 --- a/coderd/templateversions.go +++ b/coderd/templateversions.go @@ -1040,10 +1040,11 @@ func (api *API) postArchiveTemplateVersions(rw http.ResponseWriter, r *http.Requ template = httpmw.TemplateParam(r) auditor = *api.Auditor.Load() aReq, commitAudit = audit.InitRequest[database.Template](rw, &audit.RequestParams{ - Audit: auditor, - Log: api.Logger, - Request: r, - Action: database.AuditActionWrite, + Audit: auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionWrite, + OrganizationID: template.OrganizationID, }) ) defer commitAudit() @@ -1122,10 +1123,11 @@ func (api *API) setArchiveTemplateVersion(archive bool) func(rw http.ResponseWri templateVersion = httpmw.TemplateVersionParam(r) auditor = *api.Auditor.Load() aReq, commitAudit = audit.InitRequest[database.TemplateVersion](rw, &audit.RequestParams{ - Audit: auditor, - Log: api.Logger, - Request: r, - Action: database.AuditActionWrite, + Audit: auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionWrite, + OrganizationID: templateVersion.OrganizationID, }) ) defer commitAudit() @@ -1207,10 +1209,11 @@ func (api *API) patchActiveTemplateVersion(rw http.ResponseWriter, r *http.Reque template = httpmw.TemplateParam(r) auditor = *api.Auditor.Load() aReq, commitAudit = audit.InitRequest[database.Template](rw, &audit.RequestParams{ - Audit: auditor, - Log: api.Logger, - Request: r, - Action: database.AuditActionWrite, + Audit: auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionWrite, + OrganizationID: template.OrganizationID, }) ) defer commitAudit() @@ -1310,10 +1313,11 @@ func (api *API) postTemplateVersionsByOrganization(rw http.ResponseWriter, r *ht organization = httpmw.OrganizationParam(r) auditor = *api.Auditor.Load() aReq, commitAudit = audit.InitRequest[database.TemplateVersion](rw, &audit.RequestParams{ - Audit: auditor, - Log: api.Logger, - Request: r, - Action: database.AuditActionCreate, + Audit: auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionCreate, + OrganizationID: organization.ID, }) req codersdk.CreateTemplateVersionRequest diff --git a/coderd/workspaces.go b/coderd/workspaces.go index c185f6a900..0ab1e5ee41 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -345,6 +345,7 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req Request: r, Action: database.AuditActionCreate, AdditionalFields: wriBytes, + OrganizationID: organization.ID, }) defer commitAudit() @@ -644,10 +645,11 @@ func (api *API) patchWorkspace(rw http.ResponseWriter, r *http.Request) { workspace = httpmw.WorkspaceParam(r) auditor = api.Auditor.Load() aReq, commitAudit = audit.InitRequest[database.Workspace](rw, &audit.RequestParams{ - Audit: *auditor, - Log: api.Logger, - Request: r, - Action: database.AuditActionWrite, + Audit: *auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionWrite, + OrganizationID: workspace.OrganizationID, }) ) defer commitAudit() @@ -734,10 +736,11 @@ func (api *API) putWorkspaceAutostart(rw http.ResponseWriter, r *http.Request) { workspace = httpmw.WorkspaceParam(r) auditor = api.Auditor.Load() aReq, commitAudit = audit.InitRequest[database.Workspace](rw, &audit.RequestParams{ - Audit: *auditor, - Log: api.Logger, - Request: r, - Action: database.AuditActionWrite, + Audit: *auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionWrite, + OrganizationID: workspace.OrganizationID, }) ) defer commitAudit() @@ -808,10 +811,11 @@ func (api *API) putWorkspaceTTL(rw http.ResponseWriter, r *http.Request) { workspace = httpmw.WorkspaceParam(r) auditor = api.Auditor.Load() aReq, commitAudit = audit.InitRequest[database.Workspace](rw, &audit.RequestParams{ - Audit: *auditor, - Log: api.Logger, - Request: r, - Action: database.AuditActionWrite, + Audit: *auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionWrite, + OrganizationID: workspace.OrganizationID, }) ) defer commitAudit() @@ -896,10 +900,11 @@ func (api *API) putWorkspaceDormant(rw http.ResponseWriter, r *http.Request) { oldWorkspace = workspace auditor = api.Auditor.Load() aReq, commitAudit = audit.InitRequest[database.Workspace](rw, &audit.RequestParams{ - Audit: *auditor, - Log: api.Logger, - Request: r, - Action: database.AuditActionWrite, + Audit: *auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionWrite, + OrganizationID: workspace.OrganizationID, }) ) aReq.Old = oldWorkspace @@ -1094,10 +1099,11 @@ func (api *API) putFavoriteWorkspace(rw http.ResponseWriter, r *http.Request) { } aReq, commitAudit := audit.InitRequest[database.Workspace](rw, &audit.RequestParams{ - Audit: *auditor, - Log: api.Logger, - Request: r, - Action: database.AuditActionWrite, + Audit: *auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionWrite, + OrganizationID: workspace.OrganizationID, }) defer commitAudit() aReq.Old = workspace @@ -1140,10 +1146,11 @@ func (api *API) deleteFavoriteWorkspace(rw http.ResponseWriter, r *http.Request) } aReq, commitAudit := audit.InitRequest[database.Workspace](rw, &audit.RequestParams{ - Audit: *auditor, - Log: api.Logger, - Request: r, - Action: database.AuditActionWrite, + Audit: *auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionWrite, + OrganizationID: workspace.OrganizationID, }) defer commitAudit() @@ -1178,10 +1185,11 @@ func (api *API) putWorkspaceAutoupdates(rw http.ResponseWriter, r *http.Request) workspace = httpmw.WorkspaceParam(r) auditor = api.Auditor.Load() aReq, commitAudit = audit.InitRequest[database.Workspace](rw, &audit.RequestParams{ - Audit: *auditor, - Log: api.Logger, - Request: r, - Action: database.AuditActionWrite, + Audit: *auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionWrite, + OrganizationID: workspace.OrganizationID, }) ) defer commitAudit() diff --git a/enterprise/coderd/groups.go b/enterprise/coderd/groups.go index e9ae63c176..fa5da3f782 100644 --- a/enterprise/coderd/groups.go +++ b/enterprise/coderd/groups.go @@ -34,10 +34,11 @@ func (api *API) postGroupByOrganization(rw http.ResponseWriter, r *http.Request) org = httpmw.OrganizationParam(r) auditor = api.AGPL.Auditor.Load() aReq, commitAudit = audit.InitRequest[database.AuditableGroup](rw, &audit.RequestParams{ - Audit: *auditor, - Log: api.Logger, - Request: r, - Action: database.AuditActionCreate, + Audit: *auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionCreate, + OrganizationID: org.ID, }) ) defer commitAudit() @@ -97,10 +98,11 @@ func (api *API) patchGroup(rw http.ResponseWriter, r *http.Request) { group = httpmw.GroupParam(r) auditor = api.AGPL.Auditor.Load() aReq, commitAudit = audit.InitRequest[database.AuditableGroup](rw, &audit.RequestParams{ - Audit: *auditor, - Log: api.Logger, - Request: r, - Action: database.AuditActionWrite, + Audit: *auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionWrite, + OrganizationID: group.OrganizationID, }) ) defer commitAudit() @@ -299,10 +301,11 @@ func (api *API) deleteGroup(rw http.ResponseWriter, r *http.Request) { group = httpmw.GroupParam(r) auditor = api.AGPL.Auditor.Load() aReq, commitAudit = audit.InitRequest[database.AuditableGroup](rw, &audit.RequestParams{ - Audit: *auditor, - Log: api.Logger, - Request: r, - Action: database.AuditActionDelete, + Audit: *auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionDelete, + OrganizationID: group.OrganizationID, }) ) defer commitAudit() diff --git a/enterprise/coderd/templates.go b/enterprise/coderd/templates.go index 5ce27b8f80..f7ad7b12ed 100644 --- a/enterprise/coderd/templates.go +++ b/enterprise/coderd/templates.go @@ -161,10 +161,11 @@ func (api *API) patchTemplateACL(rw http.ResponseWriter, r *http.Request) { template = httpmw.TemplateParam(r) auditor = api.AGPL.Auditor.Load() aReq, commitAudit = audit.InitRequest[database.Template](rw, &audit.RequestParams{ - Audit: *auditor, - Log: api.Logger, - Request: r, - Action: database.AuditActionWrite, + Audit: *auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionWrite, + OrganizationID: template.OrganizationID, }) ) defer commitAudit()