Skip to content

Commit ecd43f1

Browse files
DanielleMaywoodDanielleMaywood
committed
fix: show audit logs for forgot password flow
1 parent c42f487 commit ecd43f1

File tree

14 files changed

+91
-41
lines changed

14 files changed

+91
-41
lines changed

‎coderd/apidoc/docs.go

+4-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

+4-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/audit.go

+14-4
Original file line numberDiff line numberDiff line change
@@ -274,8 +274,15 @@ func (api *API) convertAuditLog(ctx context.Context, dblog database.GetAuditLogs
274274

275275
func auditLogDescription(alog database.GetAuditLogsOffsetRow) string {
276276
b := strings.Builder{}
277+
277278
// NOTE: WriteString always returns a nil error, so we never check it
278-
_, _ = b.WriteString("{user} ")
279+
280+
// Requesting a one-time passcode can be performed by anyone that knows the email
281+
// of a user so saying the user performed this action might be slightly misleading.
282+
if alog.AuditLog.Action != database.AuditActionRequestOneTimePasscode {
283+
_, _ = b.WriteString("{user} ")
284+
}
285+
279286
if alog.AuditLog.StatusCode >= 400 {
280287
_, _ = b.WriteString("unsuccessfully attempted to ")
281288
_, _ = b.WriteString(string(alog.AuditLog.Action))
@@ -298,13 +305,16 @@ func auditLogDescription(alog database.GetAuditLogsOffsetRow) string {
298305
return b.String()
299306
}
300307

301-
_, _ = b.WriteString(" ")
302-
_, _ = b.WriteString(codersdk.ResourceType(alog.AuditLog.ResourceType).FriendlyString())
308+
if alog.AuditLog.Action == database.AuditActionRequestOneTimePasscode {
309+
_, _ = b.WriteString(" for")
310+
} else {
311+
_, _ = b.WriteString(" ")
312+
_, _ = b.WriteString(codersdk.ResourceType(alog.AuditLog.ResourceType).FriendlyString())
313+
}
303314

304315
if alog.AuditLog.ResourceType == database.ResourceTypeConvertLogin {
305316
_, _ = b.WriteString(" to")
306317
}
307-
308318
_, _ = b.WriteString(" {target}")
309319

310320
return b.String()

‎coderd/database/dump.sql

+2-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/migrations/000268_add_audit_action_forgot_password.down.sql

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
-- It's not possible to drop enum values from enum types, so the UP has "IF NOT
2+
-- EXISTS".

‎coderd/database/migrations/000268_add_audit_action_forgot_password.up.sql

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
ALTER TYPE audit_action
2+
ADD VALUE IF NOT EXISTS 'request_one_time_passcode';

‎coderd/database/models.go

+12-9
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/pubsub/psmock/psmock.go

+2-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/userauth.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ func (api *API) postRequestOneTimePasscode(rw http.ResponseWriter, r *http.Reque
220220
Audit: *auditor,
221221
Log: api.Logger,
222222
Request: r,
223-
Action: database.AuditActionWrite,
223+
Action: database.AuditActionRequestOneTimePasscode,
224224
})
225225
)
226226
defer commitAudit()
@@ -253,6 +253,7 @@ func (api *API) postRequestOneTimePasscode(rw http.ResponseWriter, r *http.Reque
253253
}
254254
// We continue if err == sql.ErrNoRows to help prevent a timing-based attack.
255255
aReq.Old = user
256+
aReq.UserID = user.ID
256257

257258
passcode := uuid.New()
258259
passcodeExpiresAt := dbtime.Now().Add(api.OneTimePasscodeValidityPeriod)
@@ -365,6 +366,7 @@ func (api *API) postChangePasswordWithOneTimePasscode(rw http.ResponseWriter, r
365366
}
366367
// We continue if err == sql.ErrNoRows to help prevent a timing-based attack.
367368
aReq.Old = user
369+
aReq.UserID = user.ID
368370

369371
equal, err := userpassword.Compare(string(user.HashedOneTimePasscode), req.OneTimePasscode)
370372
if err != nil {

‎codersdk/audit.go

+11-8
Original file line numberDiff line numberDiff line change
@@ -86,14 +86,15 @@ func (r ResourceType) FriendlyString() string {
8686
type AuditAction string
8787

8888
const (
89-
AuditActionCreate AuditAction = "create"
90-
AuditActionWrite AuditAction = "write"
91-
AuditActionDelete AuditAction = "delete"
92-
AuditActionStart AuditAction = "start"
93-
AuditActionStop AuditAction = "stop"
94-
AuditActionLogin AuditAction = "login"
95-
AuditActionLogout AuditAction = "logout"
96-
AuditActionRegister AuditAction = "register"
89+
AuditActionCreate AuditAction = "create"
90+
AuditActionWrite AuditAction = "write"
91+
AuditActionDelete AuditAction = "delete"
92+
AuditActionStart AuditAction = "start"
93+
AuditActionStop AuditAction = "stop"
94+
AuditActionLogin AuditAction = "login"
95+
AuditActionLogout AuditAction = "logout"
96+
AuditActionRegister AuditAction = "register"
97+
AuditActionRequestOneTimePasscode AuditAction = "request_one_time_passcode"
9798
)
9899

99100
func (a AuditAction) Friendly() string {
@@ -114,6 +115,8 @@ func (a AuditAction) Friendly() string {
114115
return "logged out"
115116
case AuditActionRegister:
116117
return "registered"
118+
case AuditActionRequestOneTimePasscode:
119+
return "one time passcode requested"
117120
default:
118121
return "unknown"
119122
}

‎docs/reference/api/schemas.md

+11-10
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎enterprise/audit/table.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ var auditableResourcesTypes = map[any]map[string]Action{
145145
"theme_preference": ActionIgnore,
146146
"name": ActionTrack,
147147
"github_com_user_id": ActionIgnore,
148-
"hashed_one_time_passcode": ActionSecret, // Do not expose a user's one time passcode.
148+
"hashed_one_time_passcode": ActionIgnore,
149149
"one_time_passcode_expires_at": ActionTrack,
150150
"must_reset_password": ActionTrack,
151151
},

‎site/src/api/typesGenerated.ts

+2-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎site/src/pages/AuditPage/AuditLogRow/AuditLogDiff/AuditLogDiff.tsx

+21
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,14 @@ const getDiffValue = (value: unknown): string => {
99
return `"${value}"`;
1010
}
1111

12+
if (isTimeObject(value)) {
13+
if (!value.Valid) {
14+
return "null";
15+
}
16+
17+
return new Date(value.Time).toLocaleString();
18+
}
19+
1220
if (Array.isArray(value)) {
1321
const values = value.map((v) => getDiffValue(v));
1422
return `[${values.join(", ")}]`;
@@ -21,6 +29,19 @@ const getDiffValue = (value: unknown): string => {
2129
return String(value);
2230
};
2331

32+
const isTimeObject = (
33+
value: unknown,
34+
): value is { Time: string; Valid: boolean } => {
35+
return (
36+
value !== null &&
37+
typeof value === "object" &&
38+
"Time" in value &&
39+
typeof value.Time === "string" &&
40+
"Valid" in value &&
41+
typeof value.Valid === "boolean"
42+
);
43+
};
44+
2445
interface AuditLogDiffProps {
2546
diff: AuditDiff;
2647
}

0 commit comments

Comments
 (0)