Skip to content

Commit 805a753

Browse files
mafredrimafredri
committed
wip auditor
1 parent 477e618 commit 805a753

File tree

4 files changed

+218
-17
lines changed

4 files changed

+218
-17
lines changed

‎coderd/audit/request.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,7 @@ func InitRequest[T Auditable](w http.ResponseWriter, p *RequestParams) (*Request
423423
action = req.Action
424424
}
425425

426-
ip := parseIP(p.Request.RemoteAddr)
426+
ip := ParseIP(p.Request.RemoteAddr)
427427
auditLog := database.AuditLog{
428428
ID: uuid.New(),
429429
Time: dbtime.Now(),
@@ -454,7 +454,7 @@ func InitRequest[T Auditable](w http.ResponseWriter, p *RequestParams) (*Request
454454
// BackgroundAudit creates an audit log for a background event.
455455
// The audit log is committed upon invocation.
456456
func BackgroundAudit[T Auditable](ctx context.Context, p *BackgroundAuditParams[T]) {
457-
ip := parseIP(p.IP)
457+
ip := ParseIP(p.IP)
458458

459459
diff := Diff(p.Audit, p.Old, p.New)
460460
var err error
@@ -567,7 +567,7 @@ func either[T Auditable, R any](old, new T, fn func(T) R, auditAction database.A
567567
panic("both old and new are nil")
568568
}
569569

570-
func parseIP(ipStr string) pqtype.Inet {
570+
func ParseIP(ipStr string) pqtype.Inet {
571571
ip := net.ParseIP(ipStr)
572572
ipNet := net.IPNet{}
573573
if ip != nil {

‎coderd/coderd.go

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -534,16 +534,6 @@ func New(options *Options) *API {
534534
Authorizer: options.Authorizer,
535535
Logger: options.Logger,
536536
},
537-
WorkspaceAppsProvider: workspaceapps.NewDBTokenProvider(
538-
options.Logger.Named("workspaceapps"),
539-
options.AccessURL,
540-
options.Authorizer,
541-
options.Database,
542-
options.DeploymentValues,
543-
oauthConfigs,
544-
options.AgentInactiveDisconnectTimeout,
545-
options.AppSigningKeyCache,
546-
),
547537
metricsCache: metricsCache,
548538
Auditor: atomic.Pointer[audit.Auditor]{},
549539
TailnetCoordinator: atomic.Pointer[tailnet.Coordinator]{},
@@ -561,6 +551,17 @@ func New(options *Options) *API {
561551
),
562552
dbRolluper: options.DatabaseRolluper,
563553
}
554+
api.WorkspaceAppsProvider = workspaceapps.NewDBTokenProvider(
555+
options.Logger.Named("workspaceapps"),
556+
options.AccessURL,
557+
options.Authorizer,
558+
&api.Auditor,
559+
options.Database,
560+
options.DeploymentValues,
561+
oauthConfigs,
562+
options.AgentInactiveDisconnectTimeout,
563+
options.AppSigningKeyCache,
564+
)
564565

565566
f := appearance.NewDefaultFetcher(api.DeploymentValues.DocsURL.String())
566567
api.AppearanceFetcher.Store(&f)

‎coderd/workspaceapps/db.go

Lines changed: 197 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,27 +3,33 @@ package workspaceapps
33
import (
44
"context"
55
"database/sql"
6+
"encoding/json"
67
"fmt"
78
"net/http"
89
"net/url"
910
"path"
1011
"slices"
1112
"strings"
13+
"sync/atomic"
1214
"time"
1315

14-
"golang.org/x/xerrors"
15-
1616
"github.com/go-jose/go-jose/v4/jwt"
17+
"github.com/google/uuid"
18+
"github.com/sqlc-dev/pqtype"
19+
"golang.org/x/xerrors"
1720

1821
"cdr.dev/slog"
22+
"github.com/coder/coder/v2/coderd/audit"
1923
"github.com/coder/coder/v2/coderd/cryptokeys"
2024
"github.com/coder/coder/v2/coderd/database"
2125
"github.com/coder/coder/v2/coderd/database/dbauthz"
26+
"github.com/coder/coder/v2/coderd/database/dbtime"
2227
"github.com/coder/coder/v2/coderd/httpapi"
2328
"github.com/coder/coder/v2/coderd/httpmw"
2429
"github.com/coder/coder/v2/coderd/jwtutils"
2530
"github.com/coder/coder/v2/coderd/rbac"
2631
"github.com/coder/coder/v2/coderd/rbac/policy"
32+
"github.com/coder/coder/v2/coderd/tracing"
2733
"github.com/coder/coder/v2/codersdk"
2834
)
2935

@@ -35,6 +41,7 @@ type DBTokenProvider struct {
3541
// DashboardURL is the main dashboard access URL for error pages.
3642
DashboardURL *url.URL
3743
Authorizer rbac.Authorizer
44+
Auditor *atomic.Pointer[audit.Auditor]
3845
Database database.Store
3946
DeploymentValues *codersdk.DeploymentValues
4047
OAuth2Configs *httpmw.OAuth2Configs
@@ -47,6 +54,7 @@ var _ SignedTokenProvider = &DBTokenProvider{}
4754
func NewDBTokenProvider(log slog.Logger,
4855
accessURL *url.URL,
4956
authz rbac.Authorizer,
57+
auditor *atomic.Pointer[audit.Auditor],
5058
db database.Store,
5159
cfg *codersdk.DeploymentValues,
5260
oauth2Cfgs *httpmw.OAuth2Configs,
@@ -61,6 +69,7 @@ func NewDBTokenProvider(log slog.Logger,
6169
Logger: log,
6270
DashboardURL: accessURL,
6371
Authorizer: authz,
72+
Auditor: auditor,
6473
Database: db,
6574
DeploymentValues: cfg,
6675
OAuth2Configs: oauth2Cfgs,
@@ -81,6 +90,8 @@ func (p *DBTokenProvider) Issue(ctx context.Context, rw http.ResponseWriter, r *
8190
// // permissions.
8291
dangerousSystemCtx := dbauthz.AsSystemRestricted(ctx)
8392

93+
aReq := p.auditInitAutocommitRequest(ctx, rw, r)
94+
8495
appReq := issueReq.AppRequest.Normalize()
8596
err := appReq.Check()
8697
if err != nil {
@@ -111,6 +122,8 @@ func (p *DBTokenProvider) Issue(ctx context.Context, rw http.ResponseWriter, r *
111122
return nil, "", false
112123
}
113124

125+
aReq.apiKey = apiKey // Update audit request.
126+
114127
// Lookup workspace app details from DB.
115128
dbReq, err := appReq.getDatabase(dangerousSystemCtx, p.Database)
116129
if xerrors.Is(err, sql.ErrNoRows) {
@@ -123,6 +136,9 @@ func (p *DBTokenProvider) Issue(ctx context.Context, rw http.ResponseWriter, r *
123136
WriteWorkspaceApp500(p.Logger, p.DashboardURL, rw, r, &appReq, err, "get app details from database")
124137
return nil, "", false
125138
}
139+
140+
aReq.dbReq = dbReq // Update audit request.
141+
126142
token.UserID = dbReq.User.ID
127143
token.WorkspaceID = dbReq.Workspace.ID
128144
token.AgentID = dbReq.Agent.ID
@@ -133,6 +149,7 @@ func (p *DBTokenProvider) Issue(ctx context.Context, rw http.ResponseWriter, r *
133149
// Verify the user has access to the app.
134150
authed, warnings, err := p.authorizeRequest(r.Context(), authz, dbReq)
135151
if err != nil {
152+
// TODO(mafredri): Audit?
136153
WriteWorkspaceApp500(p.Logger, p.DashboardURL, rw, r, &appReq, err, "verify authz")
137154
return nil, "", false
138155
}
@@ -341,3 +358,181 @@ func (p *DBTokenProvider) authorizeRequest(ctx context.Context, roles *rbac.Subj
341358
// No checks were successful.
342359
return false, warnings, nil
343360
}
361+
362+
type auditRequest struct {
363+
time time.Time
364+
ip pqtype.Inet
365+
apiKey *database.APIKey
366+
dbReq *databaseRequest
367+
}
368+
369+
// auditInitAutocommitRequest creates a new audit session and audit log for the
370+
// given request, if one does not already exist. If an audit session already
371+
// exists, it will be updated with the current timestamp. A session is used to
372+
// reduce the number of audit logs created.
373+
//
374+
// A session is unique to the agent, app, user and users IP. If any of these
375+
// values change, a new session and audit log is created.
376+
func (p *DBTokenProvider) auditInitAutocommitRequest(ctx context.Context, w http.ResponseWriter, r *http.Request) (aReq *auditRequest) {
377+
// Get the status writer from the request context so we can figure
378+
// out the HTTP status and autocommit the audit log.
379+
sw, ok := w.(*tracing.StatusWriter)
380+
if !ok {
381+
panic("dev error: http.ResponseWriter is not *tracing.StatusWriter")
382+
}
383+
384+
aReq = &auditRequest{
385+
time: dbtime.Now(),
386+
ip: audit.ParseIP(r.RemoteAddr),
387+
}
388+
389+
// Set the commit function on the status writer to create an audit
390+
// log, this ensures that the status and response body are available.
391+
sw.Done = append(sw.Done, func() {
392+
p.Logger.Info(ctx, "workspace app audit session", slog.F("status", sw.Status), slog.F("body", string(sw.ResponseBody())), slog.F("api_key", aReq.apiKey), slog.F("db_req", aReq.dbReq))
393+
394+
if sw.Status == http.StatusSeeOther {
395+
// Redirects aren't interesting as we will capture the audit
396+
// log after the redirect.
397+
//
398+
// There's a case where we call httpmw.RedirectToLogin for
399+
// path-based apps the user doesn't have access to, in which
400+
// case the dashboard login redirect is used and we end up
401+
// not hitting the workspaceapps API again due to dashboard
402+
// showing 404. (Bug?)
403+
return
404+
}
405+
406+
if aReq.dbReq == nil {
407+
// App doesn't exist, there's information in the Request
408+
// struct but we need UUIDs for audit logging.
409+
return
410+
}
411+
412+
type additionalFields struct {
413+
audit.AdditionalFields
414+
App string `json:"app"`
415+
}
416+
appInfo := additionalFields{
417+
AdditionalFields: audit.AdditionalFields{
418+
WorkspaceOwner: aReq.dbReq.Workspace.OwnerUsername,
419+
WorkspaceName: aReq.dbReq.Workspace.Name,
420+
WorkspaceID: aReq.dbReq.Workspace.ID,
421+
},
422+
App: aReq.dbReq.AppSlugOrPort,
423+
}
424+
425+
appInfoBytes, err := json.Marshal(appInfo)
426+
if err != nil {
427+
p.Logger.Error(ctx, "marshal additional fields failed", slog.Error(err))
428+
}
429+
430+
userID := uuid.NullUUID{}
431+
if aReq.apiKey != nil {
432+
userID = uuid.NullUUID{Valid: true, UUID: aReq.apiKey.UserID}
433+
}
434+
435+
var (
436+
updatedIDs []uuid.UUID
437+
sessionID = uuid.Nil
438+
)
439+
err = p.Database.InTx(func(tx database.Store) error {
440+
// nolint:gocritic // System context is needed to write audit sessions.
441+
dangerousSystemCtx := dbauthz.AsSystemRestricted(ctx)
442+
443+
updatedIDs, err = tx.UpdateWorkspaceAppAuditSession(dangerousSystemCtx, database.UpdateWorkspaceAppAuditSessionParams{
444+
AgentID: aReq.dbReq.Agent.ID,
445+
AppID: uuid.NullUUID{Valid: aReq.dbReq.App.ID != uuid.Nil, UUID: aReq.dbReq.App.ID},
446+
UserID: userID,
447+
Ip: aReq.ip,
448+
UpdatedAt: aReq.time,
449+
StaleIntervalMS: (2 * time.Hour).Milliseconds(),
450+
})
451+
if err != nil {
452+
return xerrors.Errorf("update workspace app audit session: %w", err)
453+
}
454+
if len(updatedIDs) > 0 {
455+
// Session is valid and got updated, no need to create a new audit log.
456+
return nil
457+
}
458+
459+
sessionID, err = tx.InsertWorkspaceAppAuditSession(dangerousSystemCtx, database.InsertWorkspaceAppAuditSessionParams{
460+
AgentID: aReq.dbReq.Agent.ID,
461+
AppID: uuid.NullUUID{Valid: aReq.dbReq.App.ID != uuid.Nil, UUID: aReq.dbReq.App.ID},
462+
UserID: userID,
463+
Ip: aReq.ip,
464+
StartedAt: aReq.time,
465+
UpdatedAt: aReq.time,
466+
})
467+
if err != nil {
468+
return xerrors.Errorf("insert workspace app audit session: %w", err)
469+
}
470+
471+
return nil
472+
}, nil)
473+
if err != nil {
474+
p.Logger.Error(ctx, "update workspace app audit session failed", slog.Error(err))
475+
}
476+
477+
p.Logger.Info(ctx, "workspace app audit session", slog.F("session_id", sessionID), slog.F("status", sw.Status), slog.F("body", string(sw.ResponseBody())))
478+
479+
if sessionID == uuid.Nil {
480+
if sw.Status < 400 {
481+
// Session was updated and no error occurred, no need to
482+
// create a new audit log.
483+
return
484+
}
485+
if len(updatedIDs) > 0 {
486+
// Session was updated but an error occurred, we need to
487+
// create a new audit log.
488+
sessionID = updatedIDs[0]
489+
} else {
490+
// This shouldn't happen, but fall-back to request so it
491+
// can be correlated to _something_.
492+
sessionID = httpmw.RequestID(r)
493+
}
494+
}
495+
496+
// We use the background audit function instead of init request
497+
// here because we don't know the resource type ahead of time.
498+
// This also allows us to log unauthenticated access.
499+
auditor := *p.Auditor.Load()
500+
switch {
501+
case aReq.dbReq.App.ID != uuid.Nil:
502+
audit.BackgroundAudit(ctx, &audit.BackgroundAuditParams[database.WorkspaceApp]{
503+
Audit: auditor,
504+
Log: p.Logger,
505+
506+
Action: database.AuditActionOpen,
507+
OrganizationID: aReq.dbReq.Workspace.OrganizationID,
508+
UserID: userID.UUID,
509+
RequestID: sessionID,
510+
Time: aReq.time,
511+
Status: sw.Status,
512+
IP: aReq.ip.IPNet.IP.String(),
513+
UserAgent: r.UserAgent(),
514+
New: aReq.dbReq.App,
515+
AdditionalFields: appInfoBytes,
516+
})
517+
default:
518+
// Web terminal, port app, etc.
519+
audit.BackgroundAudit(ctx, &audit.BackgroundAuditParams[database.WorkspaceAgent]{
520+
Audit: auditor,
521+
Log: p.Logger,
522+
523+
Action: database.AuditActionOpen,
524+
OrganizationID: aReq.dbReq.Workspace.OrganizationID,
525+
UserID: userID.UUID,
526+
RequestID: sessionID,
527+
Time: aReq.time,
528+
Status: sw.Status,
529+
IP: aReq.ip.IPNet.IP.String(),
530+
UserAgent: r.UserAgent(),
531+
New: aReq.dbReq.Agent,
532+
AdditionalFields: appInfoBytes,
533+
})
534+
}
535+
})
536+
537+
return aReq
538+
}

‎coderd/workspaceapps/request.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,8 @@ type databaseRequest struct {
195195
Workspace database.Workspace
196196
// Agent is the agent that the app is running on.
197197
Agent database.WorkspaceAgent
198+
// App is the app that the user is trying to access.
199+
App database.WorkspaceApp
198200

199201
// AppURL is the resolved URL to the workspace app. This is only set for non
200202
// terminal requests.
@@ -288,6 +290,7 @@ func (r Request) getDatabase(ctx context.Context, db database.Store) (*databaseR
288290
// in the workspace or not.
289291
var (
290292
agentNameOrID = r.AgentNameOrID
293+
app database.WorkspaceApp
291294
appURL string
292295
appSharingLevel database.AppSharingLevel
293296
// First check if it's a port-based URL with an optional "s" suffix for HTTPS.
@@ -353,8 +356,9 @@ func (r Request) getDatabase(ctx context.Context, db database.Store) (*databaseR
353356
appSharingLevel = ps.ShareLevel
354357
}
355358
} else {
356-
for _, app := range apps {
357-
if app.Slug == r.AppSlugOrPort {
359+
for _, a := range apps {
360+
if a.Slug == r.AppSlugOrPort {
361+
app = a
358362
if !app.Url.Valid {
359363
return nil, xerrors.Errorf("app URL is not valid")
360364
}
@@ -410,6 +414,7 @@ func (r Request) getDatabase(ctx context.Context, db database.Store) (*databaseR
410414
User: user,
411415
Workspace: workspace,
412416
Agent: agent,
417+
App: app,
413418
AppURL: appURLParsed,
414419
AppSharingLevel: appSharingLevel,
415420
}, nil

0 commit comments

Comments
 (0)