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

chore: keep active users active in scim (#13955)#13973

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
stirby merged 1 commit intorelease/2.13frompatch/2.13.2
Jul 22, 2024
Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletionscoderd/audit/request.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -257,6 +257,26 @@ func requireOrgID[T Auditable](ctx context.Context, id uuid.UUID, log slog.Logge
return id
}

// InitRequestWithCancel returns a commit function with a boolean arg.
// If the arg is false, future calls to commit() will not create an audit log
// entry.
func InitRequestWithCancel[T Auditable](w http.ResponseWriter, p *RequestParams) (*Request[T], func(commit bool)) {
req, commitF := InitRequest[T](w, p)
cancelled := false
return req, func(commit bool) {
// Once 'commit=false' is called, block
// any future commit attempts.
if !commit {
cancelled = true
return
}
// If it was ever cancelled, block any commits
if !cancelled {
commitF()
}
}
}

// InitRequest initializes an audit log for a request. It returns a function
// that should be deferred, causing the audit log to be committed when the
// handler returns.
Expand Down
45 changes: 31 additions & 14 deletionsenterprise/coderd/scim.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -272,13 +272,14 @@ func (api *API) scimPatchUser(rw http.ResponseWriter, r *http.Request) {
}

auditor := *api.AGPL.Auditor.Load()
aReq, commitAudit := audit.InitRequest[database.User](rw, &audit.RequestParams{
aReq, commitAudit := audit.InitRequestWithCancel[database.User](rw, &audit.RequestParams{
Audit: auditor,
Log: api.Logger,
Request: r,
Action: database.AuditActionWrite,
})
defer commitAudit()

defer commitAudit(true)

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

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

var status database.UserStatus
if sUser.Active {
// The user will get transitioned to Active after logging in.
status = database.UserStatusDormant
switch dbUser.Status {
case database.UserStatusActive:
// Keep the user active
status = database.UserStatusActive
case database.UserStatusDormant, database.UserStatusSuspended:
// Move (or keep) as dormant
status = database.UserStatusDormant
default:
// If the status is unknown, just move them to dormant.
// The user will get transitioned to Active after logging in.
status = database.UserStatusDormant
}
} else {
status = database.UserStatusSuspended
}

//nolint:gocritic // needed for SCIM
userNew, err := api.Database.UpdateUserStatus(dbauthz.AsSystemRestricted(r.Context()), database.UpdateUserStatusParams{
ID: dbUser.ID,
Status: status,
UpdatedAt: dbtime.Now(),
})
if err != nil {
_ = handlerutil.WriteError(rw, err)
return
if dbUser.Status != status {
//nolint:gocritic // needed for SCIM
userNew, err := api.Database.UpdateUserStatus(dbauthz.AsSystemRestricted(r.Context()), database.UpdateUserStatusParams{
ID: dbUser.ID,
Status: status,
UpdatedAt: dbtime.Now(),
})
if err != nil {
_ = handlerutil.WriteError(rw, err)
return
}
dbUser = userNew
} else {
// Do not push an audit log if there is no change.
commitAudit(false)
}
aReq.New = userNew

aReq.New = dbUser
httpapi.Write(ctx, rw, http.StatusOK, sUser)
}
79 changes: 79 additions & 0 deletionsenterprise/coderd/scim_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -8,11 +8,13 @@ import (
"net/http"
"testing"

"github.com/golang-jwt/jwt/v4"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/coder/coder/v2/coderd/audit"
"github.com/coder/coder/v2/coderd/coderdtest"
"github.com/coder/coder/v2/coderd/coderdtest/oidctest"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/cryptorand"
Expand DownExpand Up@@ -364,5 +366,82 @@ func TestScim(t *testing.T) {
require.Len(t, userRes.Users, 1)
assert.Equal(t, codersdk.UserStatusSuspended, userRes.Users[0].Status)
})

// Create a user via SCIM, which starts as dormant.
// Log in as the user, making them active.
// Then patch the user again and the user should still be active.
t.Run("ActiveIsActive", func(t *testing.T) {
t.Parallel()

ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()

scimAPIKey := []byte("hi")

mockAudit := audit.NewMock()
fake := oidctest.NewFakeIDP(t, oidctest.WithServing())
client, _ := coderdenttest.New(t, &coderdenttest.Options{
Options: &coderdtest.Options{
Auditor: mockAudit,
OIDCConfig: fake.OIDCConfig(t, []string{}),
},
SCIMAPIKey: scimAPIKey,
AuditLogging: true,
LicenseOptions: &coderdenttest.LicenseOptions{
AccountID: "coolin",
Features: license.Features{
codersdk.FeatureSCIM: 1,
codersdk.FeatureAuditLog: 1,
},
},
})
mockAudit.ResetLogs()

// User is dormant on create
sUser := makeScimUser(t)
res, err := client.Request(ctx, "POST", "/scim/v2/Users", sUser, setScimAuth(scimAPIKey))
require.NoError(t, err)
defer res.Body.Close()
assert.Equal(t, http.StatusOK, res.StatusCode)

err = json.NewDecoder(res.Body).Decode(&sUser)
require.NoError(t, err)

// Check the audit log
aLogs := mockAudit.AuditLogs()
require.Len(t, aLogs, 1)
assert.Equal(t, database.AuditActionCreate, aLogs[0].Action)

// Verify the user is dormant
scimUser, err := client.User(ctx, sUser.UserName)
require.NoError(t, err)
require.Equal(t, codersdk.UserStatusDormant, scimUser.Status, "user starts as dormant")

// Log in as the user, making them active
//nolint:bodyclose
scimUserClient, _ := fake.Login(t, client, jwt.MapClaims{
"email": sUser.Emails[0].Value,
})
scimUser, err = scimUserClient.User(ctx, codersdk.Me)
require.NoError(t, err)
require.Equal(t, codersdk.UserStatusActive, scimUser.Status, "user should now be active")

// Patch the user
mockAudit.ResetLogs()
res, err = client.Request(ctx, "PATCH", "/scim/v2/Users/"+sUser.ID, sUser, setScimAuth(scimAPIKey))
require.NoError(t, err)
_, _ = io.Copy(io.Discard, res.Body)
_ = res.Body.Close()
assert.Equal(t, http.StatusOK, res.StatusCode)

// Should be no audit logs since there is no diff
aLogs = mockAudit.AuditLogs()
require.Len(t, aLogs, 0)

// Verify the user is still active.
scimUser, err = client.User(ctx, sUser.UserName)
require.NoError(t, err)
require.Equal(t, codersdk.UserStatusActive, scimUser.Status, "user is still active")
})
})
}
Loading
[8]ページ先頭
©2009-2025 Movatter.jp