Skip to content

Commit 6618406

Browse files
EmyrkEmyrk
committed
chore: allow CreateUser to accept multiple organizations
In a multi-org deployment, it makes more sense to allow for multiple org memberships to be assigned at create. The legacy param will still be honored.
1 parent cca4519 commit 6618406

File tree

5 files changed

+185
-59
lines changed

5 files changed

+185
-59
lines changed

‎coderd/userauth.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1436,11 +1436,11 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C
14361436
}
14371437

14381438
//nolint:gocritic
1439-
user, _, err = api.CreateUser(dbauthz.AsSystemRestricted(ctx), tx, CreateUserRequest{
1439+
user, err = api.CreateUser(dbauthz.AsSystemRestricted(ctx), tx, CreateUserRequest{
14401440
CreateUserRequest: codersdk.CreateUserRequest{
1441-
Email: params.Email,
1442-
Username: params.Username,
1443-
OrganizationID: defaultOrganization.ID,
1441+
Email: params.Email,
1442+
Username: params.Username,
1443+
OrganizationIDs: []uuid.UUID{defaultOrganization.ID},
14441444
},
14451445
LoginType: params.LoginType,
14461446
})

‎coderd/users.go

Lines changed: 60 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -186,13 +186,13 @@ func (api *API) postFirstUser(rw http.ResponseWriter, r *http.Request) {
186186
}
187187

188188
//nolint:gocritic // needed to create first user
189-
user, organizationID, err := api.CreateUser(dbauthz.AsSystemRestricted(ctx), api.Database, CreateUserRequest{
189+
user, err := api.CreateUser(dbauthz.AsSystemRestricted(ctx), api.Database, CreateUserRequest{
190190
CreateUserRequest: codersdk.CreateUserRequest{
191-
Email: createUser.Email,
192-
Username: createUser.Username,
193-
Name: createUser.Name,
194-
Password: createUser.Password,
195-
OrganizationID: defaultOrg.ID,
191+
Email: createUser.Email,
192+
Username: createUser.Username,
193+
Name: createUser.Name,
194+
Password: createUser.Password,
195+
OrganizationIDs: []uuid.UUID{defaultOrg.ID},
196196
},
197197
LoginType: database.LoginTypePassword,
198198
})
@@ -240,7 +240,7 @@ func (api *API) postFirstUser(rw http.ResponseWriter, r *http.Request) {
240240

241241
httpapi.Write(ctx, rw, http.StatusCreated, codersdk.CreateFirstUserResponse{
242242
UserID: user.ID,
243-
OrganizationID: organizationID,
243+
OrganizationID: defaultOrg.ID,
244244
})
245245
}
246246

@@ -386,6 +386,20 @@ func (api *API) postUser(rw http.ResponseWriter, r *http.Request) {
386386
return
387387
}
388388

389+
if len(req.OrganizationIDs) == 0 {
390+
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
391+
Message: fmt.Sprintf("No organization specified to place the user as a member of. It is required to specify at least one organization id to place the user in."),
392+
Detail: fmt.Sprintf("required at least 1 value for the array 'organization_ids'"),
393+
Validations: []codersdk.ValidationError{
394+
{
395+
Field: "organization_ids",
396+
Detail: "Missing values, this cannot be empty",
397+
},
398+
},
399+
})
400+
return
401+
}
402+
389403
// TODO: @emyrk Authorize the organization create if the createUser will do that.
390404

391405
_, err := api.Database.GetUserByEmailOrUsername(ctx, database.GetUserByEmailOrUsernameParams{
@@ -406,44 +420,34 @@ func (api *API) postUser(rw http.ResponseWriter, r *http.Request) {
406420
return
407421
}
408422

409-
if req.OrganizationID != uuid.Nil {
410-
// If an organization was provided, make sure it exists.
411-
_, err := api.Database.GetOrganizationByID(ctx, req.OrganizationID)
412-
if err != nil {
413-
if httpapi.Is404Error(err) {
423+
// If an organization was provided, make sure it exists.
424+
for i, orgID := range req.OrganizationIDs {
425+
var orgErr error
426+
if orgID != uuid.Nil {
427+
_, orgErr = api.Database.GetOrganizationByID(ctx, orgID)
428+
} else {
429+
var defaultOrg database.Organization
430+
defaultOrg, orgErr = api.Database.GetDefaultOrganization(ctx)
431+
if orgErr == nil {
432+
// converts uuid.Nil --> default org.ID
433+
req.OrganizationIDs[i] = defaultOrg.ID
434+
}
435+
}
436+
if orgErr != nil {
437+
if httpapi.Is404Error(orgErr) {
414438
httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{
415-
Message: fmt.Sprintf("Organization does not exist with the provided id %q.", req.OrganizationID),
439+
Message: fmt.Sprintf("Organization does not exist with the provided id %q.", orgID),
416440
})
417441
return
418442
}
419443

420444
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
421445
Message: "Internal error fetching organization.",
422-
Detail: err.Error(),
423-
})
424-
return
425-
}
426-
} else {
427-
// If no organization is provided, add the user to the default
428-
defaultOrg, err := api.Database.GetDefaultOrganization(ctx)
429-
if err != nil {
430-
if httpapi.Is404Error(err) {
431-
httpapi.Write(ctx, rw, http.StatusNotFound,
432-
codersdk.Response{
433-
Message: "Resource not found or you do not have access to this resource",
434-
Detail: "Organization not found",
435-
},
436-
)
437-
return
438-
}
439-
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
440-
Message: "Internal error fetching orgs.",
441-
Detail: err.Error(),
446+
Detail: orgErr.Error(),
442447
})
443448
return
444449
}
445450

446-
req.OrganizationID = defaultOrg.ID
447451
}
448452

449453
var loginType database.LoginType
@@ -480,7 +484,7 @@ func (api *API) postUser(rw http.ResponseWriter, r *http.Request) {
480484
return
481485
}
482486

483-
user, _, err := api.CreateUser(ctx, api.Database, CreateUserRequest{
487+
user, err := api.CreateUser(ctx, api.Database, CreateUserRequest{
484488
CreateUserRequest: req,
485489
LoginType: loginType,
486490
})
@@ -505,7 +509,7 @@ func (api *API) postUser(rw http.ResponseWriter, r *http.Request) {
505509
Users: []telemetry.User{telemetry.ConvertUser(user)},
506510
})
507511

508-
httpapi.Write(ctx, rw, http.StatusCreated, db2sdk.User(user, []uuid.UUID{req.OrganizationID}))
512+
httpapi.Write(ctx, rw, http.StatusCreated, db2sdk.User(user, req.OrganizationIDs))
509513
}
510514

511515
// @Summary Delete user
@@ -1237,18 +1241,18 @@ type CreateUserRequest struct {
12371241
SkipNotifications bool
12381242
}
12391243

1240-
func (api *API) CreateUser(ctx context.Context, store database.Store, req CreateUserRequest) (database.User, uuid.UUID, error) {
1244+
func (api *API) CreateUser(ctx context.Context, store database.Store, req CreateUserRequest) (database.User, error) {
12411245
// Ensure the username is valid. It's the caller's responsibility to ensure
12421246
// the username is valid and unique.
12431247
if usernameValid := httpapi.NameValid(req.Username); usernameValid != nil {
1244-
return database.User{}, uuid.Nil, xerrors.Errorf("invalid username %q: %w", req.Username, usernameValid)
1248+
return database.User{}, xerrors.Errorf("invalid username %q: %w", req.Username, usernameValid)
12451249
}
12461250

12471251
var user database.User
12481252
err := store.InTx(func(tx database.Store) error {
12491253
orgRoles := make([]string, 0)
12501254
// Organization is required to know where to allocate the user.
1251-
if req.OrganizationID == uuid.Nil {
1255+
if len(req.OrganizationIDs) == 0 {
12521256
return xerrors.Errorf("organization ID must be provided")
12531257
}
12541258

@@ -1293,26 +1297,30 @@ func (api *API) CreateUser(ctx context.Context, store database.Store, req Create
12931297
if err != nil {
12941298
return xerrors.Errorf("insert user gitsshkey: %w", err)
12951299
}
1296-
_, err = tx.InsertOrganizationMember(ctx, database.InsertOrganizationMemberParams{
1297-
OrganizationID: req.OrganizationID,
1298-
UserID: user.ID,
1299-
CreatedAt: dbtime.Now(),
1300-
UpdatedAt: dbtime.Now(),
1301-
// By default give them membership to the organization.
1302-
Roles: orgRoles,
1303-
})
1304-
if err != nil {
1305-
return xerrors.Errorf("create organization member: %w", err)
1300+
1301+
for _, orgID := range req.OrganizationIDs {
1302+
_, err = tx.InsertOrganizationMember(ctx, database.InsertOrganizationMemberParams{
1303+
OrganizationID: orgID,
1304+
UserID: user.ID,
1305+
CreatedAt: dbtime.Now(),
1306+
UpdatedAt: dbtime.Now(),
1307+
// By default give them membership to the organization.
1308+
Roles: orgRoles,
1309+
})
1310+
if err != nil {
1311+
return xerrors.Errorf("create organization member for %q: %w", orgID.String(), err)
1312+
}
13061313
}
1314+
13071315
return nil
13081316
}, nil)
13091317
if err != nil || req.SkipNotifications {
1310-
return user, req.OrganizationID, err
1318+
return user, err
13111319
}
13121320

13131321
userAdmins, err := findUserAdmins(ctx, store)
13141322
if err != nil {
1315-
return user, req.OrganizationID, xerrors.Errorf("find user admins: %w", err)
1323+
return user, xerrors.Errorf("find user admins: %w", err)
13161324
}
13171325

13181326
for _, u := range userAdmins {
@@ -1325,7 +1333,7 @@ func (api *API) CreateUser(ctx context.Context, store database.Store, req Create
13251333
api.Logger.Warn(ctx, "unable to notify about created user", slog.F("created_user", user.Username), slog.Error(err))
13261334
}
13271335
}
1328-
return user, req.OrganizationID, err
1336+
return user, err
13291337
}
13301338

13311339
// findUserAdmins fetches all users with user admin permission including owners.

‎codersdk/users.go

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,34 @@ type CreateUserRequest struct {
123123
// DisableLogin sets the user's login type to 'none'. This prevents the user
124124
// from being able to use a password or any other authentication method to login.
125125
// Deprecated: Set UserLoginType=LoginTypeDisabled instead.
126-
DisableLogin bool `json:"disable_login"`
127-
OrganizationID uuid.UUID `json:"organization_id" validate:"" format:"uuid"`
126+
DisableLogin bool `json:"disable_login"`
127+
// OrganizationIDs is a list of organization IDs that the user should be a member of.
128+
OrganizationIDs []uuid.UUID `json:"organization_ids" validate:"" format:"uuid"`
129+
}
130+
131+
// UnmarshalJSON implements the unmarshal for the legacy param "organization_id".
132+
// To accommodate multiple organizations, the field has been switched to a slice.
133+
// The previous field will just be appended to the slice.
134+
// Note in the previous behavior, omitting the field would result in the
135+
// default org being applied, but that is no longer the case.
136+
func (r *CreateUserRequest) UnmarshalJSON(data []byte) error {
137+
// By using a type alias, we prevent an infinite recursion when unmarshalling.
138+
// This allows us to use the default unmarshal behavior of the original type.
139+
type AliasedReq CreateUserRequest
140+
type DeprecatedCreateUserRequest struct {
141+
AliasedReq
142+
OrganizationID *uuid.UUID `json:"organization_id" format:"uuid"`
143+
}
144+
var dep DeprecatedCreateUserRequest
145+
err := json.Unmarshal(data, &dep)
146+
if err != nil {
147+
return err
148+
}
149+
*r = CreateUserRequest(dep.AliasedReq)
150+
if dep.OrganizationID != nil {
151+
r.OrganizationIDs = append(r.OrganizationIDs, *dep.OrganizationID)
152+
}
153+
return nil
128154
}
129155

130156
type UpdateUserProfileRequest struct {

‎codersdk/users_test.go

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
package codersdk_test
2+
3+
import (
4+
"encoding/json"
5+
"testing"
6+
7+
"github.com/google/uuid"
8+
"github.com/stretchr/testify/require"
9+
10+
"github.com/coder/coder/v2/codersdk"
11+
)
12+
13+
func TestDeprecatedCreateUserRequest(t *testing.T) {
14+
t.Parallel()
15+
16+
t.Run("DefaultOrganization", func(t *testing.T) {
17+
t.Parallel()
18+
19+
input := `
20+
{
21+
"email":"alice@coder.com",
22+
"password":"hunter2",
23+
"username":"alice",
24+
"name":"alice",
25+
"organization_id":"00000000-0000-0000-0000-000000000000",
26+
"disable_login":false,
27+
"login_type":"none"
28+
}
29+
`
30+
var req codersdk.CreateUserRequest
31+
err := json.Unmarshal([]byte(input), &req)
32+
require.NoError(t, err)
33+
require.Equal(t, req.Email, "alice@coder.com")
34+
require.Equal(t, req.Password, "hunter2")
35+
require.Equal(t, req.Username, "alice")
36+
require.Equal(t, req.Name, "alice")
37+
require.Equal(t, req.OrganizationIDs, []uuid.UUID{uuid.Nil})
38+
require.Equal(t, req.UserLoginType, codersdk.LoginTypeNone)
39+
})
40+
41+
t.Run("MultipleOrganizations", func(t *testing.T) {
42+
t.Parallel()
43+
44+
input := `
45+
{
46+
"email":"alice@coder.com",
47+
"password":"hunter2",
48+
"username":"alice",
49+
"name":"alice",
50+
"organization_id":"00000000-0000-0000-0000-000000000000",
51+
"organization_ids":["a618cb03-99fb-4380-adb6-aa801629a4cf","8309b0dc-44ea-435d-a9ff-72cb302835e4"],
52+
"disable_login":false,
53+
"login_type":"none"
54+
}
55+
`
56+
var req codersdk.CreateUserRequest
57+
err := json.Unmarshal([]byte(input), &req)
58+
require.NoError(t, err)
59+
require.Equal(t, req.Email, "alice@coder.com")
60+
require.Equal(t, req.Password, "hunter2")
61+
require.Equal(t, req.Username, "alice")
62+
require.Equal(t, req.Name, "alice")
63+
require.ElementsMatch(t, req.OrganizationIDs,
64+
[]uuid.UUID{
65+
uuid.Nil,
66+
uuid.MustParse("a618cb03-99fb-4380-adb6-aa801629a4cf"),
67+
uuid.MustParse("8309b0dc-44ea-435d-a9ff-72cb302835e4"),
68+
})
69+
70+
require.Equal(t, req.UserLoginType, codersdk.LoginTypeNone)
71+
})
72+
73+
t.Run("OmittedOrganizations", func(t *testing.T) {
74+
t.Parallel()
75+
76+
input := `
77+
{
78+
"email":"alice@coder.com",
79+
"password":"hunter2",
80+
"username":"alice",
81+
"name":"alice",
82+
"disable_login":false,
83+
"login_type":"none"
84+
}
85+
`
86+
var req codersdk.CreateUserRequest
87+
err := json.Unmarshal([]byte(input), &req)
88+
require.NoError(t, err)
89+
90+
require.Empty(t, req.OrganizationIDs)
91+
})
92+
}

‎enterprise/coderd/scim.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ func (api *API) scimPostUser(rw http.ResponseWriter, r *http.Request) {
232232
}
233233

234234
//nolint:gocritic // needed for SCIM
235-
dbUser, _, err = api.AGPL.CreateUser(dbauthz.AsSystemRestricted(ctx), api.Database, agpl.CreateUserRequest{
235+
dbUser, err = api.AGPL.CreateUser(dbauthz.AsSystemRestricted(ctx), api.Database, agpl.CreateUserRequest{
236236
CreateUserRequest: codersdk.CreateUserRequest{
237237
Username: sUser.UserName,
238238
Email: email,

0 commit comments

Comments
 (0)