Skip to content

Add audit links/kira pilot #5156

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Dec 2, 2022
130 changes: 115 additions & 15 deletions coderd/audit.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package coderd

import (
"context"
"database/sql"
"encoding/json"
"fmt"
Expand All @@ -13,7 +14,9 @@ import (

"github.com/google/uuid"
"github.com/tabbed/pqtype"
"golang.org/x/xerrors"

"cdr.dev/slog"
"github.com/coder/coder/coderd/database"
"github.com/coder/coder/coderd/httpapi"
"github.com/coder/coder/coderd/httpmw"
Expand Down Expand Up @@ -69,7 +72,7 @@ func (api *API) auditLogs(rw http.ResponseWriter, r *http.Request) {
}

httpapi.Write(ctx, rw, http.StatusOK, codersdk.AuditLogResponse{
AuditLogs: convertAuditLogs(dblogs),
AuditLogs: api.convertAuditLogs(ctx, dblogs),
Count: dblogs[0].Count,
})
}
Expand Down Expand Up @@ -147,17 +150,17 @@ func (api *API) generateFakeAuditLog(rw http.ResponseWriter, r *http.Request) {
rw.WriteHeader(http.StatusNoContent)
}

func convertAuditLogs(dblogs []database.GetAuditLogsOffsetRow) []codersdk.AuditLog {
func (api *API) convertAuditLogs(ctx context.Context, dblogs []database.GetAuditLogsOffsetRow) []codersdk.AuditLog {
alogs := make([]codersdk.AuditLog, 0, len(dblogs))

for _, dblog := range dblogs {
alogs = append(alogs, convertAuditLog(dblog))
alogs = append(alogs, api.convertAuditLog(ctx, dblog))
}

return alogs
}

func convertAuditLog(dblog database.GetAuditLogsOffsetRow) codersdk.AuditLog {
func (api *API) convertAuditLog(ctx context.Context, dblog database.GetAuditLogsOffsetRow) codersdk.AuditLog {
ip, _ := netip.AddrFromSlice(dblog.Ip.IPNet.IP)

diff := codersdk.AuditDiff{}
Expand All @@ -182,6 +185,14 @@ func convertAuditLog(dblog database.GetAuditLogsOffsetRow) codersdk.AuditLog {
}
}

isDeleted := api.auditLogIsResourceDeleted(ctx, dblog)
var resourceLink string
if isDeleted {
resourceLink = ""
} else {
resourceLink = api.auditLogResourceLink(ctx, dblog)
}

return codersdk.AuditLog{
ID: dblog.ID,
RequestID: dblog.RequestID,
Expand All @@ -197,34 +208,123 @@ func convertAuditLog(dblog database.GetAuditLogsOffsetRow) codersdk.AuditLog {
Diff: diff,
StatusCode: dblog.StatusCode,
AdditionalFields: dblog.AdditionalFields,
Description: auditLogDescription(dblog),
User: user,
Description: auditLogDescription(dblog),
ResourceLink: resourceLink,
IsDeleted: isDeleted,
}
}

func auditLogDescription(alog database.GetAuditLogsOffsetRow) string {
str := fmt.Sprintf("{user} %s %s",
str := fmt.Sprintf("{user} %s",
codersdk.AuditAction(alog.Action).FriendlyString(),
codersdk.ResourceType(alog.ResourceType).FriendlyString(),
)

// Strings for workspace_builds follow the below format:
// "{user} started workspace build for {target}"
// where target is a workspace instead of the workspace build,
// Strings for starting/stopping workspace builds follow the below format:
// "{user} started build for workspace {target}"
// where target is a workspace instead of a workspace build
// passed in on the FE via AuditLog.AdditionalFields rather than derived in request.go:35
if alog.ResourceType == database.ResourceTypeWorkspaceBuild {
str += " for"
if alog.ResourceType == database.ResourceTypeWorkspaceBuild && alog.Action != database.AuditActionDelete {
str += " build for"
}

// We don't display the name for git ssh keys. It's fairly long and doesn't
// We don't display the name (target) for git ssh keys. It's fairly long and doesn't
// make too much sense to display.
if alog.ResourceType != database.ResourceTypeGitSshKey {
str += " {target}"
if alog.ResourceType == database.ResourceTypeGitSshKey {
str += fmt.Sprintf(" the %s",
codersdk.ResourceType(alog.ResourceType).FriendlyString())
return str
}

str += fmt.Sprintf(" %s",
codersdk.ResourceType(alog.ResourceType).FriendlyString())

str += " {target}"

return str
}

func (api *API) auditLogIsResourceDeleted(ctx context.Context, alog database.GetAuditLogsOffsetRow) bool {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coadler I'm concerned the queries in this switch will take a while to resolve. Is there a better way to determine if a resource has been deleted?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there's a great way to do so without something hacky like reflection here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the only way to really fix this would be to return if the resource is deleted in the GetAuditLogsOffset query.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per our Slack convo, will try to add that as a fast follow!

switch alog.ResourceType {
case database.ResourceTypeTemplate:
template, err := api.Database.GetTemplateByID(ctx, alog.ResourceID)
if err != nil {
if xerrors.Is(err, sql.ErrNoRows) {
return true
}
api.Logger.Error(ctx, "fetch template", slog.Error(err))
}
return template.Deleted
case database.ResourceTypeUser:
user, err := api.Database.GetUserByID(ctx, alog.ResourceID)
if err != nil {
if xerrors.Is(err, sql.ErrNoRows) {
return true
}
api.Logger.Error(ctx, "fetch user", slog.Error(err))
}
return user.Deleted
case database.ResourceTypeWorkspace:
workspace, err := api.Database.GetWorkspaceByID(ctx, alog.ResourceID)
if err != nil {
if xerrors.Is(err, sql.ErrNoRows) {
return true
}
api.Logger.Error(ctx, "fetch workspace", slog.Error(err))
}
return workspace.Deleted
case database.ResourceTypeWorkspaceBuild:
workspaceBuild, err := api.Database.GetWorkspaceBuildByID(ctx, alog.ResourceID)
if err != nil {
if xerrors.Is(err, sql.ErrNoRows) {
return true
}
api.Logger.Error(ctx, "fetch workspace build", slog.Error(err))
}
// We use workspace as a proxy for workspace build here
workspace, err := api.Database.GetWorkspaceByID(ctx, workspaceBuild.WorkspaceID)
if err != nil {
if xerrors.Is(err, sql.ErrNoRows) {
return true
}
api.Logger.Error(ctx, "fetch workspace", slog.Error(err))
}
return workspace.Deleted
default:
return false
}
}

type AdditionalFields struct {
WorkspaceName string
BuildNumber string
}

func (api *API) auditLogResourceLink(ctx context.Context, alog database.GetAuditLogsOffsetRow) string {
switch alog.ResourceType {
case database.ResourceTypeTemplate:
return fmt.Sprintf("/templates/%s",
alog.ResourceTarget)
case database.ResourceTypeUser:
return fmt.Sprintf("/users?filter=%s",
alog.ResourceTarget)
case database.ResourceTypeWorkspace:
return fmt.Sprintf("/@%s/%s",
alog.UserUsername.String, alog.ResourceTarget)
case database.ResourceTypeWorkspaceBuild:
additionalFieldsBytes := []byte(alog.AdditionalFields)
var additionalFields AdditionalFields
err := json.Unmarshal(additionalFieldsBytes, &additionalFields)
if err != nil {
api.Logger.Error(ctx, "unmarshal workspace name", slog.Error(err))
}
return fmt.Sprintf("/@%s/%s/builds/%s",
alog.UserUsername.String, additionalFields.WorkspaceName, additionalFields.BuildNumber)
default:
return ""
}
}

// auditSearchQuery takes a query string and returns the auditLog filter.
// It also can return the list of validation errors to return to the api.
func auditSearchQuery(query string) (database.GetAuditLogsOffsetParams, []codersdk.ValidationError) {
Expand Down
25 changes: 15 additions & 10 deletions coderd/audit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"testing"
"time"

"github.com/google/uuid"
"github.com/stretchr/testify/require"

"github.com/coder/coder/coderd/coderdtest"
Expand All @@ -20,9 +19,11 @@ func TestAuditLogs(t *testing.T) {

ctx := context.Background()
client := coderdtest.New(t, nil)
_ = coderdtest.CreateFirstUser(t, client)
user := coderdtest.CreateFirstUser(t, client)

err := client.CreateTestAuditLog(ctx, codersdk.CreateTestAuditLogRequest{})
err := client.CreateTestAuditLog(ctx, codersdk.CreateTestAuditLogRequest{
ResourceID: user.UserID,
})
require.NoError(t, err)

alogs, err := client.AuditLogs(ctx, codersdk.AuditLogsRequest{
Expand All @@ -43,22 +44,26 @@ func TestAuditLogsFilter(t *testing.T) {
t.Run("Filter", func(t *testing.T) {
t.Parallel()

ctx := context.Background()
client := coderdtest.New(t, nil)
_ = coderdtest.CreateFirstUser(t, client)
userResourceID := uuid.New()
var (
ctx = context.Background()
client = coderdtest.New(t, nil)
user = coderdtest.CreateFirstUser(t, client)
version = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
template = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
)

// Create two logs with "Create"
err := client.CreateTestAuditLog(ctx, codersdk.CreateTestAuditLogRequest{
Action: codersdk.AuditActionCreate,
ResourceType: codersdk.ResourceTypeTemplate,
ResourceID: template.ID,
Time: time.Date(2022, 8, 15, 14, 30, 45, 100, time.UTC), // 2022-8-15 14:30:45
})
require.NoError(t, err)
err = client.CreateTestAuditLog(ctx, codersdk.CreateTestAuditLogRequest{
Action: codersdk.AuditActionCreate,
ResourceType: codersdk.ResourceTypeUser,
ResourceID: userResourceID,
ResourceID: user.UserID,
Time: time.Date(2022, 8, 16, 14, 30, 45, 100, time.UTC), // 2022-8-16 14:30:45
})
require.NoError(t, err)
Expand All @@ -67,7 +72,7 @@ func TestAuditLogsFilter(t *testing.T) {
err = client.CreateTestAuditLog(ctx, codersdk.CreateTestAuditLogRequest{
Action: codersdk.AuditActionDelete,
ResourceType: codersdk.ResourceTypeUser,
ResourceID: userResourceID,
ResourceID: user.UserID,
Time: time.Date(2022, 8, 15, 14, 30, 45, 100, time.UTC), // 2022-8-15 14:30:45
})
require.NoError(t, err)
Expand Down Expand Up @@ -110,7 +115,7 @@ func TestAuditLogsFilter(t *testing.T) {
},
{
Name: "FilterByResourceID",
SearchQuery: "resource_id:" + userResourceID.String(),
SearchQuery: "resource_id:" + user.UserID.String(),
ExpectedResult: 2,
},
{
Expand Down
11 changes: 7 additions & 4 deletions coderd/provisionerdserver/provisionerdserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"net/http"
"net/url"
"reflect"
"strconv"
"sync"
"sync/atomic"
"time"
Expand Down Expand Up @@ -537,10 +538,11 @@ func (server *Server) FailJob(ctx context.Context, failJob *proto.FailedJob) (*p
if getWorkspaceErr != nil {
server.Logger.Error(ctx, "failed to create audit log - get workspace err", slog.Error(err))
} else {
// We pass the workspace name to the Auditor so that it
// can form a friendly string for the user.
// We pass the below information to the Auditor so that it
// can form a friendly string for the user to view in the UI.
workspaceResourceInfo := map[string]string{
"workspaceName": workspace.Name,
"buildNumber": strconv.FormatInt(int64(build.BuildNumber), 10),
}

wriBytes, err := json.Marshal(workspaceResourceInfo)
Expand Down Expand Up @@ -752,10 +754,11 @@ func (server *Server) CompleteJob(ctx context.Context, completed *proto.Complete
auditor := server.Auditor.Load()
auditAction := auditActionFromTransition(workspaceBuild.Transition)

// We pass the workspace name to the Auditor so that it
// can form a friendly string for the user.
// We pass the below information to the Auditor so that it
// can form a friendly string for the user to view in the UI.
workspaceResourceInfo := map[string]string{
"workspaceName": workspace.Name,
"buildNumber": strconv.FormatInt(int64(workspaceBuild.BuildNumber), 10),
}

wriBytes, err := json.Marshal(workspaceResourceInfo)
Expand Down
6 changes: 5 additions & 1 deletion codersdk/audit.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ func (r ResourceType) FriendlyString() string {
case ResourceTypeWorkspace:
return "workspace"
case ResourceTypeWorkspaceBuild:
return "workspace build"
// workspace builds have a unique friendly string
// see coderd/audit.go:298 for explanation
return "workspace"
case ResourceTypeGitSSHKey:
return "git ssh key"
case ResourceTypeAPIKey:
Expand Down Expand Up @@ -102,6 +104,8 @@ type AuditLog struct {
StatusCode int32 `json:"status_code"`
AdditionalFields json.RawMessage `json:"additional_fields"`
Description string `json:"description"`
ResourceLink string `json:"resource_link"`
IsDeleted bool `json:"is_deleted"`

User *User `json:"user"`
}
Expand Down
2 changes: 2 additions & 0 deletions site/src/api/typesGenerated.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ export interface AuditLog {
readonly status_code: number
readonly additional_fields: Record<string, string>
readonly description: string
readonly resource_link: string
readonly is_deleted: boolean
readonly user?: User
}

Expand Down
49 changes: 49 additions & 0 deletions site/src/components/AuditLogRow/AuditLogDescription.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import {
MockAuditLog,
MockAuditLogWithWorkspaceBuild,
} from "testHelpers/entities"
import { AuditLogDescription } from "./AuditLogDescription"
import { render } from "../../testHelpers/renderHelpers"
import { screen } from "@testing-library/react"

const getByTextContent = (text: string) => {
return screen.getByText((_, element) => {
const hasText = (element: Element | null) => element?.textContent === text
const elementHasText = hasText(element)
const childrenDontHaveText = Array.from(element?.children || []).every(
(child) => !hasText(child),
)
return elementHasText && childrenDontHaveText
})
}
describe("AuditLogDescription", () => {
it("renders the correct string for a workspace create audit log", async () => {
render(<AuditLogDescription auditLog={MockAuditLog} />)

expect(
getByTextContent("TestUser created workspace bruno-dev"),
).toBeDefined()
})

it("renders the correct string for a workspace_build stop audit log", async () => {
render(<AuditLogDescription auditLog={MockAuditLogWithWorkspaceBuild} />)

expect(
getByTextContent("TestUser stopped build for workspace test2"),
).toBeDefined()
})

it("renders the correct string for a workspace_build audit log with a duplicate word", async () => {
const AuditLogWithRepeat = {
...MockAuditLogWithWorkspaceBuild,
additional_fields: {
workspaceName: "workspace",
},
}
render(<AuditLogDescription auditLog={AuditLogWithRepeat} />)

expect(
getByTextContent("TestUser stopped build for workspace workspace"),
).toBeDefined()
})
})
Loading