diff --git a/coderd/idpsync/idpsync.go b/coderd/idpsync/idpsync.go index 1e8b14956b652..8faa22333bdc6 100644 --- a/coderd/idpsync/idpsync.go +++ b/coderd/idpsync/idpsync.go @@ -24,6 +24,7 @@ import ( // claims to the internal representation of a user in Coder. // TODO: Move group + role sync into this interface. type IDPSync interface { + AssignDefaultOrganization() bool OrganizationSyncEnabled() bool // ParseOrganizationClaims takes claims from an OIDC provider, and returns the // organization sync params for assigning users into organizations. diff --git a/coderd/idpsync/organization.go b/coderd/idpsync/organization.go index fa091eba441ad..3e2a0f84d5e5e 100644 --- a/coderd/idpsync/organization.go +++ b/coderd/idpsync/organization.go @@ -32,6 +32,10 @@ func (AGPLIDPSync) OrganizationSyncEnabled() bool { return false } +func (s AGPLIDPSync) AssignDefaultOrganization() bool { + return s.OrganizationAssignDefault +} + func (s AGPLIDPSync) ParseOrganizationClaims(_ context.Context, _ jwt.MapClaims) (OrganizationParams, *HTTPError) { // For AGPL we only sync the default organization. return OrganizationParams{ diff --git a/enterprise/coderd/scim.go b/enterprise/coderd/scim.go index b6733bbd4c052..45390b6014a6a 100644 --- a/enterprise/coderd/scim.go +++ b/enterprise/coderd/scim.go @@ -217,14 +217,19 @@ func (api *API) scimPostUser(rw http.ResponseWriter, r *http.Request) { sUser.UserName = codersdk.UsernameFrom(sUser.UserName) } - // TODO: This is a temporary solution that does not support multi-org - // deployments. This assumption places all new SCIM users into the - // default organization. - //nolint:gocritic - defaultOrganization, err := api.Database.GetDefaultOrganization(dbauthz.AsSystemRestricted(ctx)) - if err != nil { - _ = handlerutil.WriteError(rw, err) - return + // If organization sync is enabled, the user's organizations will be + // corrected on login. If including the default org, then always assign + // the default org, regardless if sync is enabled or not. + // This is to preserve single org deployment behavior. + organizations := []uuid.UUID{} + if api.IDPSync.AssignDefaultOrganization() { + //nolint:gocritic // SCIM operations are a system user + defaultOrganization, err := api.Database.GetDefaultOrganization(dbauthz.AsSystemRestricted(ctx)) + if err != nil { + _ = handlerutil.WriteError(rw, err) + return + } + organizations = append(organizations, defaultOrganization.ID) } //nolint:gocritic // needed for SCIM @@ -232,7 +237,7 @@ func (api *API) scimPostUser(rw http.ResponseWriter, r *http.Request) { CreateUserRequestWithOrgs: codersdk.CreateUserRequestWithOrgs{ Username: sUser.UserName, Email: email, - OrganizationIDs: []uuid.UUID{defaultOrganization.ID}, + OrganizationIDs: organizations, }, LoginType: database.LoginTypeOIDC, // Do not send notifications to user admins as SCIM endpoint might be called sequentially to all users. diff --git a/enterprise/coderd/scim_test.go b/enterprise/coderd/scim_test.go index 13b7aa5adca70..8d65d9bb34531 100644 --- a/enterprise/coderd/scim_test.go +++ b/enterprise/coderd/scim_test.go @@ -157,6 +157,66 @@ func TestScim(t *testing.T) { require.Len(t, userRes.Users, 1) assert.Equal(t, sUser.Emails[0].Value, userRes.Users[0].Email) assert.Equal(t, sUser.UserName, userRes.Users[0].Username) + assert.Len(t, userRes.Users[0].OrganizationIDs, 1) + + // Expect zero notifications (SkipNotifications = true) + require.Empty(t, notifyEnq.Sent) + }) + + t.Run("OKNoDefault", func(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + // given + scimAPIKey := []byte("hi") + mockAudit := audit.NewMock() + notifyEnq := &testutil.FakeNotificationsEnqueuer{} + dv := coderdtest.DeploymentValues(t) + dv.OIDC.OrganizationAssignDefault = false + client, _ := coderdenttest.New(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + Auditor: mockAudit, + NotificationsEnqueuer: notifyEnq, + DeploymentValues: dv, + }, + SCIMAPIKey: scimAPIKey, + AuditLogging: true, + LicenseOptions: &coderdenttest.LicenseOptions{ + AccountID: "coolin", + Features: license.Features{ + codersdk.FeatureSCIM: 1, + codersdk.FeatureAuditLog: 1, + }, + }, + }) + mockAudit.ResetLogs() + + // when + sUser := makeScimUser(t) + res, err := client.Request(ctx, "POST", "/scim/v2/Users", sUser, setScimAuth(scimAPIKey)) + require.NoError(t, err) + defer res.Body.Close() + require.Equal(t, http.StatusOK, res.StatusCode) + + // then + // Expect audit logs + aLogs := mockAudit.AuditLogs() + require.Len(t, aLogs, 1) + af := map[string]string{} + err = json.Unmarshal([]byte(aLogs[0].AdditionalFields), &af) + require.NoError(t, err) + assert.Equal(t, coderd.SCIMAuditAdditionalFields, af) + assert.Equal(t, database.AuditActionCreate, aLogs[0].Action) + + // Expect users exposed over API + userRes, err := client.Users(ctx, codersdk.UsersRequest{Search: sUser.Emails[0].Value}) + require.NoError(t, err) + require.Len(t, userRes.Users, 1) + assert.Equal(t, sUser.Emails[0].Value, userRes.Users[0].Email) + assert.Equal(t, sUser.UserName, userRes.Users[0].Username) + assert.Len(t, userRes.Users[0].OrganizationIDs, 0) // Expect zero notifications (SkipNotifications = true) require.Empty(t, notifyEnq.Sent)