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

Commit5b12020

Browse files
stirbyEmyrk
andauthored
chore: keep active users active in scim (#13955) (#13973)
* 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(cherry picked from commit03c5d42)Co-authored-by: Steven Masley <Emyrk@users.noreply.github.com>
1 parent5edfccf commit5b12020

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
@@ -257,6 +257,26 @@ func requireOrgID[T Auditable](ctx context.Context, id uuid.UUID, log slog.Logge
257257
returnid
258258
}
259259

260+
// InitRequestWithCancel returns a commit function with a boolean arg.
261+
// If the arg is false, future calls to commit() will not create an audit log
262+
// entry.
263+
funcInitRequestWithCancel[TAuditable](w http.ResponseWriter,p*RequestParams) (*Request[T],func(commitbool)) {
264+
req,commitF:=InitRequest[T](w,p)
265+
cancelled:=false
266+
returnreq,func(commitbool) {
267+
// Once 'commit=false' is called, block
268+
// any future commit attempts.
269+
if!commit {
270+
cancelled=true
271+
return
272+
}
273+
// If it was ever cancelled, block any commits
274+
if!cancelled {
275+
commitF()
276+
}
277+
}
278+
}
279+
260280
// InitRequest initializes an audit log for a request. It returns a function
261281
// that should be deferred, causing the audit log to be committed when the
262282
// 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