Skip to content

Commit 1f60942

Browse files
authored
chore: use legacy "AssignDefault" option for legacy behavior in SCIM (#14696)
* chore: reference legacy assign default option for legacy behavior AssignDefault is a boolean flag mainly for single org and legacy deployments. Use this flag to determine SCIM behavior.
1 parent 126a560 commit 1f60942

File tree

4 files changed

+82
-5
lines changed

4 files changed

+82
-5
lines changed

coderd/idpsync/idpsync.go

+1
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
// claims to the internal representation of a user in Coder.
2525
// TODO: Move group + role sync into this interface.
2626
type IDPSync interface {
27+
AssignDefaultOrganization() bool
2728
OrganizationSyncEnabled() bool
2829
// ParseOrganizationClaims takes claims from an OIDC provider, and returns the
2930
// organization sync params for assigning users into organizations.

coderd/idpsync/organization.go

+4
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@ func (AGPLIDPSync) OrganizationSyncEnabled() bool {
3232
return false
3333
}
3434

35+
func (s AGPLIDPSync) AssignDefaultOrganization() bool {
36+
return s.OrganizationAssignDefault
37+
}
38+
3539
func (s AGPLIDPSync) ParseOrganizationClaims(_ context.Context, _ jwt.MapClaims) (OrganizationParams, *HTTPError) {
3640
// For AGPL we only sync the default organization.
3741
return OrganizationParams{

enterprise/coderd/scim.go

+18-5
Original file line numberDiff line numberDiff line change
@@ -217,14 +217,27 @@ func (api *API) scimPostUser(rw http.ResponseWriter, r *http.Request) {
217217
sUser.UserName = codersdk.UsernameFrom(sUser.UserName)
218218
}
219219

220+
// If organization sync is enabled, the user's organizations will be
221+
// corrected on login. If including the default org, then always assign
222+
// the default org, regardless if sync is enabled or not.
223+
// This is to preserve single org deployment behavior.
224+
organizations := []uuid.UUID{}
225+
if api.IDPSync.AssignDefaultOrganization() {
226+
//nolint:gocritic // SCIM operations are a system user
227+
defaultOrganization, err := api.Database.GetDefaultOrganization(dbauthz.AsSystemRestricted(ctx))
228+
if err != nil {
229+
_ = handlerutil.WriteError(rw, err)
230+
return
231+
}
232+
organizations = append(organizations, defaultOrganization.ID)
233+
}
234+
220235
//nolint:gocritic // needed for SCIM
221236
dbUser, err = api.AGPL.CreateUser(dbauthz.AsSystemRestricted(ctx), api.Database, agpl.CreateUserRequest{
222237
CreateUserRequestWithOrgs: codersdk.CreateUserRequestWithOrgs{
223-
Username: sUser.UserName,
224-
Email: email,
225-
// In the multi-org world, SCIM does not assign any orgs. Users will
226-
// be automatically sync'd with the correct organization on login.
227-
OrganizationIDs: []uuid.UUID{},
238+
Username: sUser.UserName,
239+
Email: email,
240+
OrganizationIDs: organizations,
228241
},
229242
LoginType: database.LoginTypeOIDC,
230243
// Do not send notifications to user admins as SCIM endpoint might be called sequentially to all users.

enterprise/coderd/scim_test.go

+59
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,65 @@ func TestScim(t *testing.T) {
151151
assert.Equal(t, coderd.SCIMAuditAdditionalFields, af)
152152
assert.Equal(t, database.AuditActionCreate, aLogs[0].Action)
153153

154+
// Expect users exposed over API
155+
userRes, err := client.Users(ctx, codersdk.UsersRequest{Search: sUser.Emails[0].Value})
156+
require.NoError(t, err)
157+
require.Len(t, userRes.Users, 1)
158+
assert.Equal(t, sUser.Emails[0].Value, userRes.Users[0].Email)
159+
assert.Equal(t, sUser.UserName, userRes.Users[0].Username)
160+
assert.Len(t, userRes.Users[0].OrganizationIDs, 1)
161+
162+
// Expect zero notifications (SkipNotifications = true)
163+
require.Empty(t, notifyEnq.Sent)
164+
})
165+
166+
t.Run("OKNoDefault", func(t *testing.T) {
167+
t.Parallel()
168+
169+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
170+
defer cancel()
171+
172+
// given
173+
scimAPIKey := []byte("hi")
174+
mockAudit := audit.NewMock()
175+
notifyEnq := &testutil.FakeNotificationsEnqueuer{}
176+
dv := coderdtest.DeploymentValues(t)
177+
dv.OIDC.OrganizationAssignDefault = false
178+
client, _ := coderdenttest.New(t, &coderdenttest.Options{
179+
Options: &coderdtest.Options{
180+
Auditor: mockAudit,
181+
NotificationsEnqueuer: notifyEnq,
182+
DeploymentValues: dv,
183+
},
184+
SCIMAPIKey: scimAPIKey,
185+
AuditLogging: true,
186+
LicenseOptions: &coderdenttest.LicenseOptions{
187+
AccountID: "coolin",
188+
Features: license.Features{
189+
codersdk.FeatureSCIM: 1,
190+
codersdk.FeatureAuditLog: 1,
191+
},
192+
},
193+
})
194+
mockAudit.ResetLogs()
195+
196+
// when
197+
sUser := makeScimUser(t)
198+
res, err := client.Request(ctx, "POST", "/scim/v2/Users", sUser, setScimAuth(scimAPIKey))
199+
require.NoError(t, err)
200+
defer res.Body.Close()
201+
require.Equal(t, http.StatusOK, res.StatusCode)
202+
203+
// then
204+
// Expect audit logs
205+
aLogs := mockAudit.AuditLogs()
206+
require.Len(t, aLogs, 1)
207+
af := map[string]string{}
208+
err = json.Unmarshal([]byte(aLogs[0].AdditionalFields), &af)
209+
require.NoError(t, err)
210+
assert.Equal(t, coderd.SCIMAuditAdditionalFields, af)
211+
assert.Equal(t, database.AuditActionCreate, aLogs[0].Action)
212+
154213
// Expect users exposed over API
155214
userRes, err := client.Users(ctx, codersdk.UsersRequest{Search: sUser.Emails[0].Value})
156215
require.NoError(t, err)

0 commit comments

Comments
 (0)