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
Prev Previous commit
Next Next commit
added deleted label
  • Loading branch information
Kira-Pilot committed Nov 30, 2022
commit d5eb06a13867370ef8f729cf9ebf88af8948a636
85 changes: 39 additions & 46 deletions coderd/audit.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,10 +199,40 @@ func (api *API) convertAuditLog(ctx context.Context, dblog database.GetAuditLogs
Diff: diff,
StatusCode: dblog.StatusCode,
AdditionalFields: dblog.AdditionalFields,
Description: auditLogDescription(dblog),
User: user,
Description: auditLogDescription(dblog),
ResourceLink: api.auditLogResourceLink(ctx, dblog),
Copy link
Contributor

Choose a reason for hiding this comment

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

Because this calls api.auditLogIsResourceDeleted as well, you are making the same database calls twice here. One way to speed this up and save the work would be to call api.auditLogIsResourceDeleted before this struct and pass it into both IsDeleted and also as an arg into api.auditLogResourceLink.

Copy link
Member Author

Choose a reason for hiding this comment

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

Woops, you're totally right! Pushing a fix now.

IsDeleted: api.auditLogIsResourceDeleted(ctx, dblog),
}
}

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

// 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 && alog.Action != database.AuditActionDelete {
str += " build for"
}

// 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 += fmt.Sprintf(" the %s",
codersdk.ResourceType(alog.ResourceType).FriendlyString())
return str
}

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

str += " {target}"

return str
}

type AdditionalFields struct {
Expand All @@ -216,19 +246,19 @@ func (api *API) auditLogIsResourceDeleted(ctx context.Context, alog database.Get
case database.ResourceTypeTemplate:
template, err := api.Database.GetTemplateByID(ctx, alog.ResourceID)
if err != nil {
api.Logger.Error(ctx, "could not get template", slog.Error(err))
api.Logger.Error(ctx, "could not fetch template", slog.Error(err))
}
return template.Deleted
case database.ResourceTypeUser:
user, err := api.Database.GetUserByID(ctx, alog.ResourceID)
if err != nil {
api.Logger.Error(ctx, "could not get user", slog.Error(err))
api.Logger.Error(ctx, "could not fetch user", slog.Error(err))
Copy link
Contributor

Choose a reason for hiding this comment

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

small nit; normally we don't prefix log lines with could not/failed to because the error level implies it

}
return user.Deleted
case database.ResourceTypeWorkspace:
workspace, err := api.Database.GetWorkspaceByID(ctx, alog.ResourceID)
if err != nil {
api.Logger.Error(ctx, "could not get workspace", slog.Error(err))
api.Logger.Error(ctx, "could not fetch workspace", slog.Error(err))
}
return workspace.Deleted
case database.ResourceTypeWorkspaceBuild:
Expand All @@ -245,7 +275,7 @@ func (api *API) auditLogIsResourceDeleted(ctx context.Context, alog database.Get
// We use workspace as a proxy for workspace build here
workspace, err := api.Database.GetWorkspaceByID(ctx, uuid.MustParse(additionalFields.WorkspaceID))
if err != nil {
api.Logger.Error(ctx, "could not get workspace", slog.Error(err))
api.Logger.Error(ctx, "could not fetch workspace", slog.Error(err))
}
return workspace.Deleted
default:
Expand All @@ -254,29 +284,21 @@ func (api *API) auditLogIsResourceDeleted(ctx context.Context, alog database.Get
}

func (api *API) auditLogResourceLink(ctx context.Context, alog database.GetAuditLogsOffsetRow) string {
if api.auditLogIsResourceDeleted(ctx, alog) {
return ""
}

switch alog.ResourceType {
case database.ResourceTypeTemplate:
if api.auditLogIsResourceDeleted(ctx, alog) {
return ""
}
return fmt.Sprintf("/templates/%s",
alog.ResourceTarget)
case database.ResourceTypeUser:
if api.auditLogIsResourceDeleted(ctx, alog) {
return ""
}
return fmt.Sprintf("/users?filter=%s",
alog.ResourceTarget)
case database.ResourceTypeWorkspace:
if api.auditLogIsResourceDeleted(ctx, alog) {
return ""
}
return fmt.Sprintf("/@%s/%s",
alog.UserUsername.String, alog.ResourceTarget)
case database.ResourceTypeWorkspaceBuild:
if api.auditLogIsResourceDeleted(ctx, alog) {
return ""
}
additionalFieldsBytes := []byte(alog.AdditionalFields)
var additionalFields AdditionalFields
err := json.Unmarshal(additionalFieldsBytes, &additionalFields)
Expand All @@ -290,35 +312,6 @@ func (api *API) auditLogResourceLink(ctx context.Context, alog database.GetAudit
}
}

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

// 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 && alog.Action != database.AuditActionDelete {
str += " build for"
}

// 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 += fmt.Sprintf(" the %s",
codersdk.ResourceType(alog.ResourceType).FriendlyString())
return str
}

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

str += " {target}"

return str
}

// 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
1 change: 1 addition & 0 deletions codersdk/audit.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ type AuditLog struct {
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
1 change: 1 addition & 0 deletions site/src/api/typesGenerated.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ export interface AuditLog {
readonly additional_fields: Record<string, string>
readonly description: string
readonly resource_link: string
readonly is_deleted: boolean
readonly user?: User
}

Expand Down
17 changes: 17 additions & 0 deletions site/src/components/AuditLogRow/AuditLogDescription.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,15 @@ import { FC } from "react"
import { AuditLog } from "api/typesGenerated"
import { Link as RouterLink } from "react-router-dom"
import Link from "@material-ui/core/Link"
import { makeStyles } from "@material-ui/core/styles"
import i18next from "i18next"

export const AuditLogDescription: FC<{ auditLog: AuditLog }> = ({
auditLog,
}): JSX.Element => {
const classes = useStyles()
const { t } = i18next

let target = auditLog.resource_target.trim()

// audit logs with a resource_type of workspace build use workspace name as a target
Expand Down Expand Up @@ -41,6 +46,18 @@ export const AuditLogDescription: FC<{ auditLog: AuditLog }> = ({
) : (
<strong>{target}</strong>
)}
{auditLog.is_deleted && (
<span className={classes.deletedLabel}>
<> {t("auditLog:table.logRow.deletedLabel")}</>
</span>
)}
</span>
)
}

const useStyles = makeStyles((theme) => ({
deletedLabel: {
...theme.typography.caption,
color: theme.palette.text.secondary,
},
}))
1 change: 1 addition & 0 deletions site/src/i18n/en/auditLog.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"emptyPage": "No audit logs available on this page",
"noLogs": "No audit logs available",
"logRow": {
"deletedLabel": "(deleted)",
"ip": "IP: ",
"os": "OS: ",
"browser": "Browser: ",
Expand Down