diff --git a/coderd/audit/request.go b/coderd/audit/request.go index e6d9d01fbfd27..048231382d6a6 100644 --- a/coderd/audit/request.go +++ b/coderd/audit/request.go @@ -233,6 +233,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. diff --git a/enterprise/coderd/scim.go b/enterprise/coderd/scim.go index ca3f19fce2d3a..726d76794f75d 100644 --- a/enterprise/coderd/scim.go +++ b/enterprise/coderd/scim.go @@ -273,21 +273,34 @@ 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 - _, 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 } httpapi.Write(ctx, rw, http.StatusOK, sUser) diff --git a/enterprise/coderd/scim_test.go b/enterprise/coderd/scim_test.go index 95d297605e1fc..f89661f59cefb 100644 --- a/enterprise/coderd/scim_test.go +++ b/enterprise/coderd/scim_test.go @@ -8,9 +8,12 @@ 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/coderdtest" + "github.com/coder/coder/v2/coderd/coderdtest/oidctest" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/cryptorand" "github.com/coder/coder/v2/enterprise/coderd" @@ -338,5 +341,68 @@ 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") + + fake := oidctest.NewFakeIDP(t, oidctest.WithServing()) + client, _ := coderdenttest.New(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + OIDCConfig: fake.OIDCConfig(t, []string{}), + }, + SCIMAPIKey: scimAPIKey, + AuditLogging: true, + LicenseOptions: &coderdenttest.LicenseOptions{ + AccountID: "coolin", + Features: license.Features{ + codersdk.FeatureSCIM: 1, + }, + }, + }) + + // 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) + + // 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 + 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) + + // 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") + }) }) }