Skip to content

Commit f6c89a2

Browse files
Kira-Pilotcoadler
Kira-Pilot
and
coadler
authored
feat: differentiate new user registration from user login in the audit log (#7096)
* auditing register events * fix tests * update docs * update comments * Update coderd/audit/request.go Co-authored-by: Colin Adler <colin1adler@gmail.com> --------- Co-authored-by: Colin Adler <colin1adler@gmail.com>
1 parent d1d459c commit f6c89a2

15 files changed

+100
-53
lines changed

‎coderd/apidoc/docs.go

Lines changed: 4 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

Lines changed: 12 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/audit.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -247,9 +247,9 @@ func auditLogDescription(alog database.GetAuditLogsOffsetRow) string {
247247
)
248248

249249
// API Key resources (used for authentication) do not have targets and follow the below format:
250-
// "User {logged in | logged out}"
250+
// "User {logged in | logged out | registered}"
251251
if alog.ResourceType == database.ResourceTypeApiKey &&
252-
(alog.Action == database.AuditActionLogin || alog.Action == database.AuditActionLogout) {
252+
(alog.Action == database.AuditActionLogin || alog.Action == database.AuditActionLogout || alog.Action == database.AuditActionRegister) {
253253
return str
254254
}
255255

‎coderd/audit/request.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ type Request[T Auditable] struct {
3636
// This optional field can be passed in when the userID cannot be determined from the API Key
3737
// such as in the case of login, when the audit log is created prior the API Key's existence.
3838
UserID uuid.UUID
39+
40+
// This optional field can be passed in if the AuditAction must be overridden
41+
// such as in the case of new user authentication when the Audit Action is 'register', not 'login'.
42+
Action database.AuditAction
3943
}
4044

4145
type BuildAuditParams[T Auditable] struct {
@@ -198,6 +202,11 @@ func InitRequest[T Auditable](w http.ResponseWriter, p *RequestParams) (*Request
198202
return
199203
}
200204

205+
action := p.Action
206+
if req.Action != "" {
207+
action = req.Action
208+
}
209+
201210
ip := parseIP(p.Request.RemoteAddr)
202211
auditLog := database.AuditLog{
203212
ID: uuid.New(),
@@ -208,7 +217,7 @@ func InitRequest[T Auditable](w http.ResponseWriter, p *RequestParams) (*Request
208217
ResourceType: either(req.Old, req.New, ResourceType[T], req.params.Action),
209218
ResourceID: either(req.Old, req.New, ResourceID[T], req.params.Action),
210219
ResourceTarget: either(req.Old, req.New, ResourceTarget[T], req.params.Action),
211-
Action: p.Action,
220+
Action: action,
212221
Diff: diffRaw,
213222
StatusCode: int32(sw.Status),
214223
RequestID: httpmw.RequestID(p.Request),

‎coderd/database/dump.sql

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/migrations/000117_add_audit_action_register.down.sql

Lines changed: 2 additions & 0 deletions
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/000117_add_audit_action_register.up.sql

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
ALTER TYPE audit_action
2+
ADD VALUE IF NOT EXISTS 'register';

‎coderd/database/models.go

Lines changed: 11 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/userauth.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,12 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
395395
return
396396
}
397397

398+
// If a new user is authenticating for the first time
399+
// the audit action is 'register', not 'login'
400+
if user.ID == uuid.Nil {
401+
aReq.Action = database.AuditActionRegister
402+
}
403+
398404
cookie, key, err := api.oauthLogin(r, oauthLoginParams{
399405
User: user,
400406
Link: link,
@@ -712,6 +718,12 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
712718
return
713719
}
714720

721+
// If a new user is authenticating for the first time
722+
// the audit action is 'register', not 'login'
723+
if user.ID == uuid.Nil {
724+
aReq.Action = database.AuditActionRegister
725+
}
726+
715727
cookie, key, err := api.oauthLogin(r, oauthLoginParams{
716728
User: user,
717729
Link: link,

‎coderd/userauth_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ func TestUserOAuth2Github(t *testing.T) {
261261

262262
require.Len(t, auditor.AuditLogs(), numLogs)
263263
require.NotEqual(t, auditor.AuditLogs()[numLogs-1].UserID, uuid.Nil)
264-
require.Equal(t, database.AuditActionLogin, auditor.AuditLogs()[numLogs-1].Action)
264+
require.Equal(t, database.AuditActionRegister, auditor.AuditLogs()[numLogs-1].Action)
265265
})
266266
t.Run("SignupAllowedTeam", func(t *testing.T) {
267267
t.Parallel()
@@ -305,7 +305,7 @@ func TestUserOAuth2Github(t *testing.T) {
305305

306306
require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode)
307307
require.Len(t, auditor.AuditLogs(), numLogs)
308-
require.Equal(t, database.AuditActionLogin, auditor.AuditLogs()[numLogs-1].Action)
308+
require.Equal(t, database.AuditActionRegister, auditor.AuditLogs()[numLogs-1].Action)
309309
})
310310
t.Run("SignupAllowedTeamInFirstOrganization", func(t *testing.T) {
311311
t.Parallel()
@@ -357,7 +357,7 @@ func TestUserOAuth2Github(t *testing.T) {
357357

358358
require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode)
359359
require.Len(t, auditor.AuditLogs(), numLogs)
360-
require.Equal(t, database.AuditActionLogin, auditor.AuditLogs()[numLogs-1].Action)
360+
require.Equal(t, database.AuditActionRegister, auditor.AuditLogs()[numLogs-1].Action)
361361
})
362362
t.Run("SignupAllowedTeamInSecondOrganization", func(t *testing.T) {
363363
t.Parallel()
@@ -409,7 +409,7 @@ func TestUserOAuth2Github(t *testing.T) {
409409

410410
require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode)
411411
require.Len(t, auditor.AuditLogs(), numLogs)
412-
require.Equal(t, database.AuditActionLogin, auditor.AuditLogs()[numLogs-1].Action)
412+
require.Equal(t, database.AuditActionRegister, auditor.AuditLogs()[numLogs-1].Action)
413413
})
414414
t.Run("SignupAllowEveryone", func(t *testing.T) {
415415
t.Parallel()
@@ -447,7 +447,7 @@ func TestUserOAuth2Github(t *testing.T) {
447447

448448
require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode)
449449
require.Len(t, auditor.AuditLogs(), numLogs)
450-
require.Equal(t, database.AuditActionLogin, auditor.AuditLogs()[numLogs-1].Action)
450+
require.Equal(t, database.AuditActionRegister, auditor.AuditLogs()[numLogs-1].Action)
451451
})
452452
t.Run("SignupFailedInactiveInOrg", func(t *testing.T) {
453453
t.Parallel()
@@ -721,7 +721,7 @@ func TestUserOIDC(t *testing.T) {
721721

722722
require.Len(t, auditor.AuditLogs(), numLogs)
723723
require.NotEqual(t, auditor.AuditLogs()[numLogs-1].UserID, uuid.Nil)
724-
require.Equal(t, database.AuditActionLogin, auditor.AuditLogs()[numLogs-1].Action)
724+
require.Equal(t, database.AuditActionRegister, auditor.AuditLogs()[numLogs-1].Action)
725725
}
726726

727727
if tc.AvatarURL != "" {
@@ -731,7 +731,7 @@ func TestUserOIDC(t *testing.T) {
731731
require.Equal(t, tc.AvatarURL, user.AvatarURL)
732732

733733
require.Len(t, auditor.AuditLogs(), numLogs)
734-
require.Equal(t, database.AuditActionLogin, auditor.AuditLogs()[numLogs-1].Action)
734+
require.Equal(t, database.AuditActionRegister, auditor.AuditLogs()[numLogs-1].Action)
735735
}
736736
})
737737
}
@@ -782,7 +782,7 @@ func TestUserOIDC(t *testing.T) {
782782
require.True(t, strings.HasPrefix(user.Username, "jon-"), "username %q should have prefix %q", user.Username, "jon-")
783783

784784
require.Len(t, auditor.AuditLogs(), numLogs)
785-
require.Equal(t, database.AuditActionLogin, auditor.AuditLogs()[numLogs-1].Action)
785+
require.Equal(t, database.AuditActionRegister, auditor.AuditLogs()[numLogs-1].Action)
786786
})
787787

788788
t.Run("Disabled", func(t *testing.T) {

‎codersdk/audit.go

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -55,13 +55,14 @@ func (r ResourceType) FriendlyString() string {
5555
type AuditAction string
5656

5757
const (
58-
AuditActionCreate AuditAction = "create"
59-
AuditActionWrite AuditAction = "write"
60-
AuditActionDelete AuditAction = "delete"
61-
AuditActionStart AuditAction = "start"
62-
AuditActionStop AuditAction = "stop"
63-
AuditActionLogin AuditAction = "login"
64-
AuditActionLogout AuditAction = "logout"
58+
AuditActionCreate AuditAction = "create"
59+
AuditActionWrite AuditAction = "write"
60+
AuditActionDelete AuditAction = "delete"
61+
AuditActionStart AuditAction = "start"
62+
AuditActionStop AuditAction = "stop"
63+
AuditActionLogin AuditAction = "login"
64+
AuditActionLogout AuditAction = "logout"
65+
AuditActionRegister AuditAction = "register"
6566
)
6667

6768
func (a AuditAction) Friendly() string {
@@ -80,6 +81,8 @@ func (a AuditAction) Friendly() string {
8081
return "logged in"
8182
case AuditActionLogout:
8283
return "logged out"
84+
case AuditActionRegister:
85+
return "registered"
8386
default:
8487
return "unknown"
8588
}

0 commit comments

Comments
 (0)