synced/coder
1
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2025-07-13 21:36:50 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat: differentiate new user registration from user login in the audit log (#7096)

Browse Source 
* 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>
This commit is contained in:
Kira Pilot
2023-04-12 11:46:16 -07:00
committed by GitHub
parent d1d459cb79
commit f6c89a2615
15 changed files with 100 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
coderd/apidoc/docs.go generated
View File

@ -6117,7 +6117,8 @@ const docTemplate = `{
"start",
"stop",
"login",
"logout"
"logout",
"register"
],
"x-enum-varnames": [
"AuditActionCreate",
@ -6126,7 +6127,8 @@ const docTemplate = `{
"AuditActionStart",
"AuditActionStop",
"AuditActionLogin",
"AuditActionLogout"
"AuditActionLogout",
"AuditActionRegister"
]
},
"codersdk.AuditDiff": {

14
coderd/apidoc/swagger.json generated
View File

@ -5433,7 +5433,16 @@
},
"codersdk.AuditAction": {
"type": "string",
"enum": ["create", "write", "delete", "start", "stop", "login", "logout"],
"enum": [
"create",
"write",
"delete",
"start",
"stop",
"login",
"logout",
"register"
],
"x-enum-varnames": [
"AuditActionCreate",
"AuditActionWrite",
@ -5441,7 +5450,8 @@
"AuditActionStart",
"AuditActionStop",
"AuditActionLogin",
"AuditActionLogout"
"AuditActionLogout",
"AuditActionRegister"
]
},
"codersdk.AuditDiff": {

4
coderd/audit.go
View File

@ -247,9 +247,9 @@ func auditLogDescription(alog database.GetAuditLogsOffsetRow) string {
)
// API Key resources (used for authentication) do not have targets and follow the below format:
// "User {logged in | logged out}"
// "User {logged in | logged out | registered}"
if alog.ResourceType == database.ResourceTypeApiKey &&
(alog.Action == database.AuditActionLogin || alog.Action == database.AuditActionLogout) {
(alog.Action == database.AuditActionLogin || alog.Action == database.AuditActionLogout || alog.Action == database.AuditActionRegister) {
return str
}

11
coderd/audit/request.go
View File

@ -36,6 +36,10 @@ type Request[T Auditable] struct {
// This optional field can be passed in when the userID cannot be determined from the API Key
// such as in the case of login, when the audit log is created prior the API Key's existence.
UserID uuid.UUID
// This optional field can be passed in if the AuditAction must be overridden
// such as in the case of new user authentication when the Audit Action is 'register', not 'login'.
Action database.AuditAction
}
type BuildAuditParams[T Auditable] struct {
@ -198,6 +202,11 @@ func InitRequest[T Auditable](w http.ResponseWriter, p *RequestParams) (*Request
return
}
action := p.Action
if req.Action != "" {
action = req.Action
}
ip := parseIP(p.Request.RemoteAddr)
auditLog := database.AuditLog{
ID: uuid.New(),
@ -208,7 +217,7 @@ func InitRequest[T Auditable](w http.ResponseWriter, p *RequestParams) (*Request
ResourceType: either(req.Old, req.New, ResourceType[T], req.params.Action),
ResourceID: either(req.Old, req.New, ResourceID[T], req.params.Action),
ResourceTarget: either(req.Old, req.New, ResourceTarget[T], req.params.Action),
Action: p.Action,
Action: action,
Diff: diffRaw,
StatusCode: int32(sw.Status),
RequestID: httpmw.RequestID(p.Request),

3
coderd/database/dump.sql generated
View File

@ -18,7 +18,8 @@ CREATE TYPE audit_action AS ENUM (
'start',
'stop',
'login',
'logout'
'logout',
'register'
);
CREATE TYPE build_reason AS ENUM (

2
coderd/database/migrations/000117_add_audit_action_register.down.sql Normal file
View File

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

2
coderd/database/migrations/000117_add_audit_action_register.up.sql Normal file
View File

@ -0,0 +1,2 @@
ALTER TYPE audit_action
ADD VALUE IF NOT EXISTS 'register';

19
coderd/database/models.go
View File

@ -139,13 +139,14 @@ func AllAppSharingLevelValues() []AppSharingLevel {
type AuditAction string
const (
AuditActionCreate AuditAction = "create"
AuditActionWrite AuditAction = "write"
AuditActionDelete AuditAction = "delete"
AuditActionStart AuditAction = "start"
AuditActionStop AuditAction = "stop"
AuditActionLogin AuditAction = "login"
AuditActionLogout AuditAction = "logout"
AuditActionCreate AuditAction = "create"
AuditActionWrite AuditAction = "write"
AuditActionDelete AuditAction = "delete"
AuditActionStart AuditAction = "start"
AuditActionStop AuditAction = "stop"
AuditActionLogin AuditAction = "login"
AuditActionLogout AuditAction = "logout"
AuditActionRegister AuditAction = "register"
)
func (e *AuditAction) Scan(src interface{}) error {
@ -191,7 +192,8 @@ func (e AuditAction) Valid() bool {
AuditActionStart,
AuditActionStop,
AuditActionLogin,
AuditActionLogout:
AuditActionLogout,
AuditActionRegister:
return true
}
return false
@ -206,6 +208,7 @@ func AllAuditActionValues() []AuditAction {
AuditActionStop,
AuditActionLogin,
AuditActionLogout,
AuditActionRegister,
}
}

12
coderd/userauth.go
View File

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

16
coderd/userauth_test.go
View File

@ -261,7 +261,7 @@ func TestUserOAuth2Github(t *testing.T) {
require.Len(t, auditor.AuditLogs(), numLogs)
require.NotEqual(t, auditor.AuditLogs()[numLogs-1].UserID, uuid.Nil)
require.Equal(t, database.AuditActionLogin, auditor.AuditLogs()[numLogs-1].Action)
require.Equal(t, database.AuditActionRegister, auditor.AuditLogs()[numLogs-1].Action)
})
t.Run("SignupAllowedTeam", func(t *testing.T) {
t.Parallel()
@ -305,7 +305,7 @@ func TestUserOAuth2Github(t *testing.T) {
require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode)
require.Len(t, auditor.AuditLogs(), numLogs)
require.Equal(t, database.AuditActionLogin, auditor.AuditLogs()[numLogs-1].Action)
require.Equal(t, database.AuditActionRegister, auditor.AuditLogs()[numLogs-1].Action)
})
t.Run("SignupAllowedTeamInFirstOrganization", func(t *testing.T) {
t.Parallel()
@ -357,7 +357,7 @@ func TestUserOAuth2Github(t *testing.T) {
require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode)
require.Len(t, auditor.AuditLogs(), numLogs)
require.Equal(t, database.AuditActionLogin, auditor.AuditLogs()[numLogs-1].Action)
require.Equal(t, database.AuditActionRegister, auditor.AuditLogs()[numLogs-1].Action)
})
t.Run("SignupAllowedTeamInSecondOrganization", func(t *testing.T) {
t.Parallel()
@ -409,7 +409,7 @@ func TestUserOAuth2Github(t *testing.T) {
require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode)
require.Len(t, auditor.AuditLogs(), numLogs)
require.Equal(t, database.AuditActionLogin, auditor.AuditLogs()[numLogs-1].Action)
require.Equal(t, database.AuditActionRegister, auditor.AuditLogs()[numLogs-1].Action)
})
t.Run("SignupAllowEveryone", func(t *testing.T) {
t.Parallel()
@ -447,7 +447,7 @@ func TestUserOAuth2Github(t *testing.T) {
require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode)
require.Len(t, auditor.AuditLogs(), numLogs)
require.Equal(t, database.AuditActionLogin, auditor.AuditLogs()[numLogs-1].Action)
require.Equal(t, database.AuditActionRegister, auditor.AuditLogs()[numLogs-1].Action)
})
t.Run("SignupFailedInactiveInOrg", func(t *testing.T) {
t.Parallel()
@ -721,7 +721,7 @@ func TestUserOIDC(t *testing.T) {
require.Len(t, auditor.AuditLogs(), numLogs)
require.NotEqual(t, auditor.AuditLogs()[numLogs-1].UserID, uuid.Nil)
require.Equal(t, database.AuditActionLogin, auditor.AuditLogs()[numLogs-1].Action)
require.Equal(t, database.AuditActionRegister, auditor.AuditLogs()[numLogs-1].Action)
}
if tc.AvatarURL != "" {
@ -731,7 +731,7 @@ func TestUserOIDC(t *testing.T) {
require.Equal(t, tc.AvatarURL, user.AvatarURL)
require.Len(t, auditor.AuditLogs(), numLogs)
require.Equal(t, database.AuditActionLogin, auditor.AuditLogs()[numLogs-1].Action)
require.Equal(t, database.AuditActionRegister, auditor.AuditLogs()[numLogs-1].Action)
}
})
}
@ -782,7 +782,7 @@ func TestUserOIDC(t *testing.T) {
require.True(t, strings.HasPrefix(user.Username, "jon-"), "username %q should have prefix %q", user.Username, "jon-")
require.Len(t, auditor.AuditLogs(), numLogs)
require.Equal(t, database.AuditActionLogin, auditor.AuditLogs()[numLogs-1].Action)
require.Equal(t, database.AuditActionRegister, auditor.AuditLogs()[numLogs-1].Action)
})
t.Run("Disabled", func(t *testing.T) {