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

Commit03c5d42

Browse files
authored
chore: keep active users active in scim (#13955)
* chore: scim should keep active users active* chore: add a unit test to excercise dormancy bug* audit log should not be dropped when there is no change* add ability to cancel audit log
1 parent49d6d0f commit03c5d42

File tree

3 files changed

+130
-14
lines changed

3 files changed

+130
-14
lines changed

‎coderd/audit/request.go‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,26 @@ func requireOrgID[T Auditable](ctx context.Context, id uuid.UUID, log slog.Logge
267267
returnid
268268
}
269269

270+
// InitRequestWithCancel returns a commit function with a boolean arg.
271+
// If the arg is false, future calls to commit() will not create an audit log
272+
// entry.
273+
funcInitRequestWithCancel[TAuditable](w http.ResponseWriter,p*RequestParams) (*Request[T],func(commitbool)) {
274+
req,commitF:=InitRequest[T](w,p)
275+
cancelled:=false
276+
returnreq,func(commitbool) {
277+
// Once 'commit=false' is called, block
278+
// any future commit attempts.
279+
if!commit {
280+
cancelled=true
281+
return
282+
}
283+
// If it was ever cancelled, block any commits
284+
if!cancelled {
285+
commitF()
286+
}
287+
}
288+
}
289+
270290
// InitRequest initializes an audit log for a request. It returns a function
271291
// that should be deferred, causing the audit log to be committed when the
272292
// handler returns.

‎enterprise/coderd/scim.go‎

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -272,13 +272,14 @@ func (api *API) scimPatchUser(rw http.ResponseWriter, r *http.Request) {
272272
}
273273

274274
auditor:=*api.AGPL.Auditor.Load()
275-
aReq,commitAudit:=audit.InitRequest[database.User](rw,&audit.RequestParams{
275+
aReq,commitAudit:=audit.InitRequestWithCancel[database.User](rw,&audit.RequestParams{
276276
Audit:auditor,
277277
Log:api.Logger,
278278
Request:r,
279279
Action:database.AuditActionWrite,
280280
})
281-
defercommitAudit()
281+
282+
defercommitAudit(true)
282283

283284
id:=chi.URLParam(r,"id")
284285

@@ -307,23 +308,39 @@ func (api *API) scimPatchUser(rw http.ResponseWriter, r *http.Request) {
307308

308309
varstatus database.UserStatus
309310
ifsUser.Active {
310-
// The user will get transitioned to Active after logging in.
311-
status=database.UserStatusDormant
311+
switchdbUser.Status {
312+
casedatabase.UserStatusActive:
313+
// Keep the user active
314+
status=database.UserStatusActive
315+
casedatabase.UserStatusDormant,database.UserStatusSuspended:
316+
// Move (or keep) as dormant
317+
status=database.UserStatusDormant
318+
default:
319+
// If the status is unknown, just move them to dormant.
320+
// The user will get transitioned to Active after logging in.
321+
status=database.UserStatusDormant
322+
}
312323
}else {
313324
status=database.UserStatusSuspended
314325
}
315326

316-
//nolint:gocritic // needed for SCIM
317-
userNew,err:=api.Database.UpdateUserStatus(dbauthz.AsSystemRestricted(r.Context()), database.UpdateUserStatusParams{
318-
ID:dbUser.ID,
319-
Status:status,
320-
UpdatedAt:dbtime.Now(),
321-
})
322-
iferr!=nil {
323-
_=handlerutil.WriteError(rw,err)
324-
return
327+
ifdbUser.Status!=status {
328+
//nolint:gocritic // needed for SCIM
329+
userNew,err:=api.Database.UpdateUserStatus(dbauthz.AsSystemRestricted(r.Context()), database.UpdateUserStatusParams{
330+
ID:dbUser.ID,
331+
Status:status,
332+
UpdatedAt:dbtime.Now(),
333+
})
334+
iferr!=nil {
335+
_=handlerutil.WriteError(rw,err)
336+
return
337+
}
338+
dbUser=userNew
339+
}else {
340+
// Do not push an audit log if there is no change.
341+
commitAudit(false)
325342
}
326-
aReq.New=userNew
327343

344+
aReq.New=dbUser
328345
httpapi.Write(ctx,rw,http.StatusOK,sUser)
329346
}

‎enterprise/coderd/scim_test.go‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,13 @@ import (
88
"net/http"
99
"testing"
1010

11+
"github.com/golang-jwt/jwt/v4"
1112
"github.com/stretchr/testify/assert"
1213
"github.com/stretchr/testify/require"
1314

1415
"github.com/coder/coder/v2/coderd/audit"
1516
"github.com/coder/coder/v2/coderd/coderdtest"
17+
"github.com/coder/coder/v2/coderd/coderdtest/oidctest"
1618
"github.com/coder/coder/v2/coderd/database"
1719
"github.com/coder/coder/v2/codersdk"
1820
"github.com/coder/coder/v2/cryptorand"
@@ -364,5 +366,82 @@ func TestScim(t *testing.T) {
364366
require.Len(t,userRes.Users,1)
365367
assert.Equal(t,codersdk.UserStatusSuspended,userRes.Users[0].Status)
366368
})
369+
370+
// Create a user via SCIM, which starts as dormant.
371+
// Log in as the user, making them active.
372+
// Then patch the user again and the user should still be active.
373+
t.Run("ActiveIsActive",func(t*testing.T) {
374+
t.Parallel()
375+
376+
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitLong)
377+
defercancel()
378+
379+
scimAPIKey:= []byte("hi")
380+
381+
mockAudit:=audit.NewMock()
382+
fake:=oidctest.NewFakeIDP(t,oidctest.WithServing())
383+
client,_:=coderdenttest.New(t,&coderdenttest.Options{
384+
Options:&coderdtest.Options{
385+
Auditor:mockAudit,
386+
OIDCConfig:fake.OIDCConfig(t, []string{}),
387+
},
388+
SCIMAPIKey:scimAPIKey,
389+
AuditLogging:true,
390+
LicenseOptions:&coderdenttest.LicenseOptions{
391+
AccountID:"coolin",
392+
Features: license.Features{
393+
codersdk.FeatureSCIM:1,
394+
codersdk.FeatureAuditLog:1,
395+
},
396+
},
397+
})
398+
mockAudit.ResetLogs()
399+
400+
// User is dormant on create
401+
sUser:=makeScimUser(t)
402+
res,err:=client.Request(ctx,"POST","/scim/v2/Users",sUser,setScimAuth(scimAPIKey))
403+
require.NoError(t,err)
404+
deferres.Body.Close()
405+
assert.Equal(t,http.StatusOK,res.StatusCode)
406+
407+
err=json.NewDecoder(res.Body).Decode(&sUser)
408+
require.NoError(t,err)
409+
410+
// Check the audit log
411+
aLogs:=mockAudit.AuditLogs()
412+
require.Len(t,aLogs,1)
413+
assert.Equal(t,database.AuditActionCreate,aLogs[0].Action)
414+
415+
// Verify the user is dormant
416+
scimUser,err:=client.User(ctx,sUser.UserName)
417+
require.NoError(t,err)
418+
require.Equal(t,codersdk.UserStatusDormant,scimUser.Status,"user starts as dormant")
419+
420+
// Log in as the user, making them active
421+
//nolint:bodyclose
422+
scimUserClient,_:=fake.Login(t,client, jwt.MapClaims{
423+
"email":sUser.Emails[0].Value,
424+
})
425+
scimUser,err=scimUserClient.User(ctx,codersdk.Me)
426+
require.NoError(t,err)
427+
require.Equal(t,codersdk.UserStatusActive,scimUser.Status,"user should now be active")
428+
429+
// Patch the user
430+
mockAudit.ResetLogs()
431+
res,err=client.Request(ctx,"PATCH","/scim/v2/Users/"+sUser.ID,sUser,setScimAuth(scimAPIKey))
432+
require.NoError(t,err)
433+
_,_=io.Copy(io.Discard,res.Body)
434+
_=res.Body.Close()
435+
assert.Equal(t,http.StatusOK,res.StatusCode)
436+
437+
// Should be no audit logs since there is no diff
438+
aLogs=mockAudit.AuditLogs()
439+
require.Len(t,aLogs,0)
440+
441+
// Verify the user is still active.
442+
scimUser,err=client.User(ctx,sUser.UserName)
443+
require.NoError(t,err)
444+
require.Equal(t,codersdk.UserStatusActive,scimUser.Status,"user is still active")
445+
})
367446
})
368447
}

0 commit comments

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