Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit46fe59f

Browse files
Kira-PilotEmyrk
andauthored
feat: audit login (#5925)
* added migration for api key resource* sort of working* auditing login* passing the correct user id* added and fixed tests* gen documentation* formatting and lint* lint* audit Github oauth and write tests* audit oauth and write tests* added defer fn for login error auditing* fixed test* feat: audit logout (#5998)* Update coderd/userauth.goCo-authored-by: Steven Masley <Emyrk@users.noreply.github.com>* fix test* bypassing diff generation if login/logout* lint---------Co-authored-by: Steven Masley <Emyrk@users.noreply.github.com>
1 parent060eeed commit46fe59f

29 files changed

+679
-259
lines changed

‎coderd/apidoc/docs.go

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

‎coderd/apidoc/swagger.json

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

‎coderd/apikey.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ func (api *API) postToken(rw http.ResponseWriter, r *http.Request) {
7070
return
7171
}
7272

73-
cookie,err:=api.createAPIKey(ctx,createAPIKeyParams{
73+
cookie,_,err:=api.createAPIKey(ctx,createAPIKeyParams{
7474
UserID:user.ID,
7575
LoginType:database.LoginTypeToken,
7676
ExpiresAt:database.Now().Add(lifeTime),
@@ -108,7 +108,7 @@ func (api *API) postAPIKey(rw http.ResponseWriter, r *http.Request) {
108108
}
109109

110110
lifeTime:=time.Hour*24*7
111-
cookie,err:=api.createAPIKey(ctx,createAPIKeyParams{
111+
cookie,_,err:=api.createAPIKey(ctx,createAPIKeyParams{
112112
UserID:user.ID,
113113
LoginType:database.LoginTypePassword,
114114
RemoteAddr:r.RemoteAddr,
@@ -281,10 +281,10 @@ func (api *API) validateAPIKeyLifetime(lifetime time.Duration) error {
281281
returnnil
282282
}
283283

284-
func (api*API)createAPIKey(ctx context.Context,paramscreateAPIKeyParams) (*http.Cookie,error) {
284+
func (api*API)createAPIKey(ctx context.Context,paramscreateAPIKeyParams) (*http.Cookie,*database.APIKey,error) {
285285
keyID,keySecret,err:=GenerateAPIKeyIDSecret()
286286
iferr!=nil {
287-
returnnil,xerrors.Errorf("generate API key: %w",err)
287+
returnnil,nil,xerrors.Errorf("generate API key: %w",err)
288288
}
289289
hashed:=sha256.Sum256([]byte(keySecret))
290290

@@ -315,7 +315,7 @@ func (api *API) createAPIKey(ctx context.Context, params createAPIKeyParams) (*h
315315
switchscope {
316316
casedatabase.APIKeyScopeAll,database.APIKeyScopeApplicationConnect:
317317
default:
318-
returnnil,xerrors.Errorf("invalid API key scope: %q",scope)
318+
returnnil,nil,xerrors.Errorf("invalid API key scope: %q",scope)
319319
}
320320

321321
key,err:=api.Database.InsertAPIKey(ctx, database.InsertAPIKeyParams{
@@ -338,7 +338,7 @@ func (api *API) createAPIKey(ctx context.Context, params createAPIKeyParams) (*h
338338
Scope:scope,
339339
})
340340
iferr!=nil {
341-
returnnil,xerrors.Errorf("insert API key: %w",err)
341+
returnnil,nil,xerrors.Errorf("insert API key: %w",err)
342342
}
343343

344344
api.Telemetry.Report(&telemetry.Snapshot{
@@ -354,5 +354,5 @@ func (api *API) createAPIKey(ctx context.Context, params createAPIKeyParams) (*h
354354
HttpOnly:true,
355355
SameSite:http.SameSiteLaxMode,
356356
Secure:api.SecureAuthCookie,
357-
},nil
357+
},&key,nil
358358
}

‎coderd/audit.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,12 @@ func auditLogDescription(alog database.GetAuditLogsOffsetRow, additionalFields a
264264
codersdk.AuditAction(alog.Action).Friendly(),
265265
)
266266

267+
// API Key resources do not have targets and follow the below format:
268+
// "User {logged in | logged out}"
269+
ifalog.ResourceType==database.ResourceTypeApiKey {
270+
returnstr
271+
}
272+
267273
// Strings for starting/stopping workspace builds follow the below format:
268274
// "{user | 'Coder automatically'} started build #{build_number} for workspace {target}"
269275
// where target is a workspace (name) instead of a workspace build
@@ -488,6 +494,10 @@ func actionFromString(actionString string) string {
488494
returnactionString
489495
casecodersdk.AuditActionStop:
490496
returnactionString
497+
casecodersdk.AuditActionLogin:
498+
returnactionString
499+
casecodersdk.AuditActionLogout:
500+
returnactionString
491501
default:
492502
}
493503
return""

‎coderd/audit/request.go

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@ type Request[T Auditable] struct {
3131

3232
OldT
3333
NewT
34+
35+
// This optional field can be passed in when the userID cannot be determined from the API Key
36+
// such as in the case of login, when the audit log is created prior the API Key's existence.
37+
UserID uuid.UUID
3438
}
3539

3640
typeBuildAuditParams[TAuditable]struct {
@@ -64,6 +68,9 @@ func ResourceTarget[T Auditable](tgt T) string {
6468
returntyped.PublicKey
6569
case database.AuditableGroup:
6670
returntyped.Group.Name
71+
case database.APIKey:
72+
// this isn't used
73+
return""
6774
default:
6875
panic(fmt.Sprintf("unknown resource %T",tgt))
6976
}
@@ -85,6 +92,8 @@ func ResourceID[T Auditable](tgt T) uuid.UUID {
8592
returntyped.UserID
8693
case database.AuditableGroup:
8794
returntyped.Group.ID
95+
case database.APIKey:
96+
returntyped.UserID
8897
default:
8998
panic(fmt.Sprintf("unknown resource %T",tgt))
9099
}
@@ -106,6 +115,8 @@ func ResourceType[T Auditable](tgt T) database.ResourceType {
106115
returndatabase.ResourceTypeGitSshKey
107116
case database.AuditableGroup:
108117
returndatabase.ResourceTypeGroup
118+
case database.APIKey:
119+
returndatabase.ResourceTypeApiKey
109120
default:
110121
panic(fmt.Sprintf("unknown resource %T",tgt))
111122
}
@@ -130,7 +141,14 @@ func InitRequest[T Auditable](w http.ResponseWriter, p *RequestParams) (*Request
130141

131142
// If no resources were provided, there's nothing we can audit.
132143
ifResourceID(req.Old)==uuid.Nil&&ResourceID(req.New)==uuid.Nil {
133-
return
144+
// If the request action is a login or logout, we always want to audit it even if
145+
// there is no diff. This is so we can capture events where an API Key is never created
146+
// because an unknown user fails to login.
147+
// TODO: introduce the concept of an anonymous user so we always have a userID even
148+
// when dealing with a mystery user. https://github.com/coder/coder/issues/6054
149+
ifreq.params.Action!=database.AuditActionLogin&&req.params.Action!=database.AuditActionLogout {
150+
return
151+
}
134152
}
135153

136154
vardiffRaw= []byte("{}")
@@ -150,16 +168,24 @@ func InitRequest[T Auditable](w http.ResponseWriter, p *RequestParams) (*Request
150168
p.AdditionalFields=json.RawMessage("{}")
151169
}
152170

171+
varuserID uuid.UUID
172+
key,ok:=httpmw.APIKeyOptional(p.Request)
173+
ifok {
174+
userID=key.UserID
175+
}else {
176+
userID=req.UserID
177+
}
178+
153179
ip:=parseIP(p.Request.RemoteAddr)
154180
auditLog:= database.AuditLog{
155181
ID:uuid.New(),
156182
Time:database.Now(),
157-
UserID:httpmw.APIKey(p.Request).UserID,
183+
UserID:userID,
158184
Ip:ip,
159185
UserAgent: sql.NullString{String:p.Request.UserAgent(),Valid:true},
160-
ResourceType:either(req.Old,req.New,ResourceType[T]),
161-
ResourceID:either(req.Old,req.New,ResourceID[T]),
162-
ResourceTarget:either(req.Old,req.New,ResourceTarget[T]),
186+
ResourceType:either(req.Old,req.New,ResourceType[T],req.params.Action),
187+
ResourceID:either(req.Old,req.New,ResourceID[T],req.params.Action),
188+
ResourceTarget:either(req.Old,req.New,ResourceTarget[T],req.params.Action),
163189
Action:p.Action,
164190
Diff:diffRaw,
165191
StatusCode:int32(sw.Status),
@@ -202,9 +228,9 @@ func BuildAudit[T Auditable](ctx context.Context, p *BuildAuditParams[T]) {
202228
UserID:p.UserID,
203229
Ip:ip,
204230
UserAgent: sql.NullString{},
205-
ResourceType:either(p.Old,p.New,ResourceType[T]),
206-
ResourceID:either(p.Old,p.New,ResourceID[T]),
207-
ResourceTarget:either(p.Old,p.New,ResourceTarget[T]),
231+
ResourceType:either(p.Old,p.New,ResourceType[T],p.Action),
232+
ResourceID:either(p.Old,p.New,ResourceID[T],p.Action),
233+
ResourceTarget:either(p.Old,p.New,ResourceTarget[T],p.Action),
208234
Action:p.Action,
209235
Diff:diffRaw,
210236
StatusCode:int32(p.Status),
@@ -221,11 +247,15 @@ func BuildAudit[T Auditable](ctx context.Context, p *BuildAuditParams[T]) {
221247
}
222248
}
223249

224-
funceither[TAuditable,Rany](old,newT,fnfunc(T)R)R {
250+
funceither[TAuditable,Rany](old,newT,fnfunc(T)R,auditAction database.AuditAction)R {
225251
ifResourceID(new)!=uuid.Nil {
226252
returnfn(new)
227253
}elseifResourceID(old)!=uuid.Nil {
228254
returnfn(old)
255+
}elseifauditAction==database.AuditActionLogin||auditAction==database.AuditActionLogout {
256+
// If the request action is a login or logout, we always want to audit it even if
257+
// there is no diff. See the comment in audit.InitRequest for more detail.
258+
returnfn(old)
229259
}else {
230260
panic("both old and new are nil")
231261
}

‎coderd/database/dump.sql

Lines changed: 3 additions & 1 deletion
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.
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".
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
ALTERTYPE audit_action
2+
ADD VALUE IF NOT EXISTS'login';
3+
4+
ALTERTYPE audit_action
5+
ADD VALUE IF NOT EXISTS'logout';
6+
7+
ALTERTYPE resource_type
8+
ADD VALUE IF NOT EXISTS'api_key';
9+

‎coderd/database/models.go

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

‎coderd/gitsshkey_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,8 @@ func TestGitSSHKey(t *testing.T) {
9595
require.NotEmpty(t,key2.PublicKey)
9696
require.NotEqual(t,key2.PublicKey,key1.PublicKey)
9797

98-
require.Len(t,auditor.AuditLogs,1)
99-
assert.Equal(t,database.AuditActionWrite,auditor.AuditLogs[0].Action)
98+
require.Len(t,auditor.AuditLogs,2)
99+
assert.Equal(t,database.AuditActionWrite,auditor.AuditLogs[1].Action)
100100
})
101101
}
102102

‎coderd/templates_test.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,11 @@ func TestPostTemplateByOrganization(t *testing.T) {
6161
assert.Equal(t,expected.Name,got.Name)
6262
assert.Equal(t,expected.Description,got.Description)
6363

64-
require.Len(t,auditor.AuditLogs,3)
65-
assert.Equal(t,database.AuditActionCreate,auditor.AuditLogs[0].Action)
66-
assert.Equal(t,database.AuditActionWrite,auditor.AuditLogs[1].Action)
67-
assert.Equal(t,database.AuditActionCreate,auditor.AuditLogs[2].Action)
64+
require.Len(t,auditor.AuditLogs,4)
65+
assert.Equal(t,database.AuditActionLogin,auditor.AuditLogs[0].Action)
66+
assert.Equal(t,database.AuditActionCreate,auditor.AuditLogs[1].Action)
67+
assert.Equal(t,database.AuditActionWrite,auditor.AuditLogs[2].Action)
68+
assert.Equal(t,database.AuditActionCreate,auditor.AuditLogs[3].Action)
6869
})
6970

7071
t.Run("AlreadyExists",func(t*testing.T) {
@@ -285,8 +286,8 @@ func TestPatchTemplateMeta(t *testing.T) {
285286
assert.Equal(t,req.DefaultTTLMillis,updated.DefaultTTLMillis)
286287
assert.False(t,req.AllowUserCancelWorkspaceJobs)
287288

288-
require.Len(t,auditor.AuditLogs,4)
289-
assert.Equal(t,database.AuditActionWrite,auditor.AuditLogs[3].Action)
289+
require.Len(t,auditor.AuditLogs,5)
290+
assert.Equal(t,database.AuditActionWrite,auditor.AuditLogs[4].Action)
290291
})
291292

292293
t.Run("NoMaxTTL",func(t*testing.T) {
@@ -448,8 +449,8 @@ func TestDeleteTemplate(t *testing.T) {
448449
err:=client.DeleteTemplate(ctx,template.ID)
449450
require.NoError(t,err)
450451

451-
require.Len(t,auditor.AuditLogs,4)
452-
assert.Equal(t,database.AuditActionDelete,auditor.AuditLogs[3].Action)
452+
require.Len(t,auditor.AuditLogs,5)
453+
assert.Equal(t,database.AuditActionDelete,auditor.AuditLogs[4].Action)
453454
})
454455

455456
t.Run("Workspaces",func(t*testing.T) {

‎coderd/templateversions_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,8 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) {
127127
require.Equal(t,"bananas",version.Name)
128128
require.Equal(t,provisionerdserver.ScopeOrganization,version.Job.Tags[provisionerdserver.TagScope])
129129

130-
require.Len(t,auditor.AuditLogs,1)
131-
assert.Equal(t,database.AuditActionCreate,auditor.AuditLogs[0].Action)
130+
require.Len(t,auditor.AuditLogs,2)
131+
assert.Equal(t,database.AuditActionCreate,auditor.AuditLogs[1].Action)
132132
})
133133
t.Run("Example",func(t*testing.T) {
134134
t.Parallel()
@@ -646,8 +646,8 @@ func TestPatchActiveTemplateVersion(t *testing.T) {
646646
})
647647
require.NoError(t,err)
648648

649-
require.Len(t,auditor.AuditLogs,4)
650-
assert.Equal(t,database.AuditActionWrite,auditor.AuditLogs[3].Action)
649+
require.Len(t,auditor.AuditLogs,5)
650+
assert.Equal(t,database.AuditActionWrite,auditor.AuditLogs[4].Action)
651651
})
652652
}
653653

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp