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.
coder · Emyrk · Aug 23, 2024 · Aug 21, 2024 · Aug 21, 2024 · Aug 21, 2024
commit d8e6672a75fea91f791387ec281237fe7a27199e
diff --git a/coderd/userauth.go b/coderd/userauth.go
@@ -1436,11 +1436,11 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C
 			}
 
 			//nolint:gocritic
-			user, _, err = api.CreateUser(dbauthz.AsSystemRestricted(ctx), tx, CreateUserRequest{
+			user, err = api.CreateUser(dbauthz.AsSystemRestricted(ctx), tx, CreateUserRequest{
 				CreateUserRequest: codersdk.CreateUserRequest{
-					Email:          params.Email,
-					Username:       params.Username,
-					OrganizationID: defaultOrganization.ID,
+					Email:           params.Email,
+					Username:        params.Username,
+					OrganizationIDs: []uuid.UUID{defaultOrganization.ID},
 				},
 				LoginType: params.LoginType,
 			})

diff --git a/coderd/users.go b/coderd/users.go
@@ -186,13 +186,13 @@ func (api *API) postFirstUser(rw http.ResponseWriter, r *http.Request) {
 	}
 
 	//nolint:gocritic // needed to create first user
-	user, organizationID, err := api.CreateUser(dbauthz.AsSystemRestricted(ctx), api.Database, CreateUserRequest{
+	user, err := api.CreateUser(dbauthz.AsSystemRestricted(ctx), api.Database, CreateUserRequest{
 		CreateUserRequest: codersdk.CreateUserRequest{
-			Email:          createUser.Email,
-			Username:       createUser.Username,
-			Name:           createUser.Name,
-			Password:       createUser.Password,
-			OrganizationID: defaultOrg.ID,
+			Email:           createUser.Email,
+			Username:        createUser.Username,
+			Name:            createUser.Name,
+			Password:        createUser.Password,
+			OrganizationIDs: []uuid.UUID{defaultOrg.ID},
 		},
 		LoginType: database.LoginTypePassword,
 	})
@@ -240,7 +240,7 @@ func (api *API) postFirstUser(rw http.ResponseWriter, r *http.Request) {
 
 	httpapi.Write(ctx, rw, http.StatusCreated, codersdk.CreateFirstUserResponse{
 		UserID:         user.ID,
-		OrganizationID: organizationID,
+		OrganizationID: defaultOrg.ID,
 	})
 }
 
@@ -386,6 +386,20 @@ func (api *API) postUser(rw http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	if len(req.OrganizationIDs) == 0 {
+		httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
+			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."),
+			Detail:  fmt.Sprintf("required at least 1 value for the array 'organization_ids'"),
+			Validations: []codersdk.ValidationError{
+				{
+					Field:  "organization_ids",
+					Detail: "Missing values, this cannot be empty",
+				},
+			},
+		})
+		return
+	}
+
 	// TODO: @emyrk Authorize the organization create if the createUser will do that.
 
 	_, err := api.Database.GetUserByEmailOrUsername(ctx, database.GetUserByEmailOrUsernameParams{
@@ -406,44 +420,34 @@ func (api *API) postUser(rw http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if req.OrganizationID != uuid.Nil {
-		// If an organization was provided, make sure it exists.
-		_, err := api.Database.GetOrganizationByID(ctx, req.OrganizationID)
-		if err != nil {
-			if httpapi.Is404Error(err) {
+	// If an organization was provided, make sure it exists.
+	for i, orgID := range req.OrganizationIDs {
+		var orgErr error
+		if orgID != uuid.Nil {
+			_, orgErr = api.Database.GetOrganizationByID(ctx, orgID)
+		} else {
+			var defaultOrg database.Organization
+			defaultOrg, orgErr = api.Database.GetDefaultOrganization(ctx)
+			if orgErr == nil {
+				// converts uuid.Nil --> default org.ID
+				req.OrganizationIDs[i] = defaultOrg.ID
+			}
+		}
+		if orgErr != nil {
+			if httpapi.Is404Error(orgErr) {
 				httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{
-					Message: fmt.Sprintf("Organization does not exist with the provided id %q.", req.OrganizationID),
+					Message: fmt.Sprintf("Organization does not exist with the provided id %q.", orgID),
 				})
 				return
 			}
 
 			httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
 				Message: "Internal error fetching organization.",
-				Detail:  err.Error(),
-			})
-			return
-		}
-	} else {
-		// If no organization is provided, add the user to the default
-		defaultOrg, err := api.Database.GetDefaultOrganization(ctx)
-		if err != nil {
-			if httpapi.Is404Error(err) {
-				httpapi.Write(ctx, rw, http.StatusNotFound,
-					codersdk.Response{
-						Message: "Resource not found or you do not have access to this resource",
-						Detail:  "Organization not found",
-					},
-				)
-				return
-			}
-			httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
-				Message: "Internal error fetching orgs.",
-				Detail:  err.Error(),
+				Detail:  orgErr.Error(),
 			})
 			return
 		}
 
-		req.OrganizationID = defaultOrg.ID
 	}
 
 	var loginType database.LoginType
@@ -480,7 +484,7 @@ func (api *API) postUser(rw http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	user, _, err := api.CreateUser(ctx, api.Database, CreateUserRequest{
+	user, err := api.CreateUser(ctx, api.Database, CreateUserRequest{
 		CreateUserRequest: req,
 		LoginType:         loginType,
 	})
@@ -505,7 +509,7 @@ func (api *API) postUser(rw http.ResponseWriter, r *http.Request) {
 		Users: []telemetry.User{telemetry.ConvertUser(user)},
 	})
 
-	httpapi.Write(ctx, rw, http.StatusCreated, db2sdk.User(user, []uuid.UUID{req.OrganizationID}))
+	httpapi.Write(ctx, rw, http.StatusCreated, db2sdk.User(user, req.OrganizationIDs))
 }
 
 // @Summary Delete user
@@ -1285,18 +1289,18 @@ type CreateUserRequest struct {
 	SkipNotifications bool
 }
 
-func (api *API) CreateUser(ctx context.Context, store database.Store, req CreateUserRequest) (database.User, uuid.UUID, error) {
+func (api *API) CreateUser(ctx context.Context, store database.Store, req CreateUserRequest) (database.User, error) {
 	// Ensure the username is valid. It's the caller's responsibility to ensure
 	// the username is valid and unique.
 	if usernameValid := httpapi.NameValid(req.Username); usernameValid != nil {
-		return database.User{}, uuid.Nil, xerrors.Errorf("invalid username %q: %w", req.Username, usernameValid)
+		return database.User{}, xerrors.Errorf("invalid username %q: %w", req.Username, usernameValid)
 	}
 
 	var user database.User
 	err := store.InTx(func(tx database.Store) error {
 		orgRoles := make([]string, 0)
 		// Organization is required to know where to allocate the user.
-		if req.OrganizationID == uuid.Nil {
+		if len(req.OrganizationIDs) == 0 {
 			return xerrors.Errorf("organization ID must be provided")
 		}
 
@@ -1341,26 +1345,30 @@ func (api *API) CreateUser(ctx context.Context, store database.Store, req Create
 		if err != nil {
 			return xerrors.Errorf("insert user gitsshkey: %w", err)
 		}
-		_, err = tx.InsertOrganizationMember(ctx, database.InsertOrganizationMemberParams{
-			OrganizationID: req.OrganizationID,
-			UserID:         user.ID,
-			CreatedAt:      dbtime.Now(),
-			UpdatedAt:      dbtime.Now(),
-			// By default give them membership to the organization.
-			Roles: orgRoles,
-		})
-		if err != nil {
-			return xerrors.Errorf("create organization member: %w", err)
+
+		for _, orgID := range req.OrganizationIDs {
+			_, err = tx.InsertOrganizationMember(ctx, database.InsertOrganizationMemberParams{
+				OrganizationID: orgID,
+				UserID:         user.ID,
+				CreatedAt:      dbtime.Now(),
+				UpdatedAt:      dbtime.Now(),
+				// By default give them membership to the organization.
+				Roles: orgRoles,
+			})
+			if err != nil {
+				return xerrors.Errorf("create organization member for %q: %w", orgID.String(), err)
+			}
 		}
+
 		return nil
 	}, nil)
 	if err != nil || req.SkipNotifications {
-		return user, req.OrganizationID, err
+		return user, err
 	}
 
 	userAdmins, err := findUserAdmins(ctx, store)
 	if err != nil {
-		return user, req.OrganizationID, xerrors.Errorf("find user admins: %w", err)
+		return user, xerrors.Errorf("find user admins: %w", err)
 	}
 
 	for _, u := range userAdmins {
@@ -1373,7 +1381,7 @@ func (api *API) CreateUser(ctx context.Context, store database.Store, req Create
 			api.Logger.Warn(ctx, "unable to notify about created user", slog.F("created_user", user.Username), slog.Error(err))
 		}
 	}
-	return user, req.OrganizationID, err
+	return user, err
 }
 
 // findUserAdmins fetches all users with user admin permission including owners.

diff --git a/codersdk/users.go b/codersdk/users.go
@@ -123,8 +123,34 @@ type CreateUserRequest struct {
 	// DisableLogin sets the user's login type to 'none'. This prevents the user
 	// from being able to use a password or any other authentication method to login.
 	// Deprecated: Set UserLoginType=LoginTypeDisabled instead.
-	DisableLogin   bool      `json:"disable_login"`
-	OrganizationID uuid.UUID `json:"organization_id" validate:"" format:"uuid"`
+	DisableLogin bool `json:"disable_login"`
+	// OrganizationIDs is a list of organization IDs that the user should be a member of.
+	OrganizationIDs []uuid.UUID `json:"organization_ids" validate:"" format:"uuid"`
+}
+
+// UnmarshalJSON implements the unmarshal for the legacy param "organization_id".
+// To accommodate multiple organizations, the field has been switched to a slice.
+// The previous field will just be appended to the slice.
+// Note in the previous behavior, omitting the field would result in the
+// default org being applied, but that is no longer the case.
+func (r *CreateUserRequest) UnmarshalJSON(data []byte) error {
+	// By using a type alias, we prevent an infinite recursion when unmarshalling.
+	// This allows us to use the default unmarshal behavior of the original type.
+	type AliasedReq CreateUserRequest
+	type DeprecatedCreateUserRequest struct {
+		AliasedReq
+		OrganizationID *uuid.UUID `json:"organization_id" format:"uuid"`
+	}
+	var dep DeprecatedCreateUserRequest
+	err := json.Unmarshal(data, &dep)
+	if err != nil {
+		return err
+	}
+	*r = CreateUserRequest(dep.AliasedReq)
+	if dep.OrganizationID != nil {
+		r.OrganizationIDs = append(r.OrganizationIDs, *dep.OrganizationID)
+	}
+	return nil
 }
 
 type UpdateUserProfileRequest struct {

diff --git a/codersdk/users_test.go b/codersdk/users_test.go
@@ -0,0 +1,92 @@
+package codersdk_test
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/google/uuid"
+	"github.com/stretchr/testify/require"
+
+	"github.com/coder/coder/v2/codersdk"
+)
+
+func TestDeprecatedCreateUserRequest(t *testing.T) {
+	t.Parallel()
+
+	t.Run("DefaultOrganization", func(t *testing.T) {
+		t.Parallel()
+
+		input := `
+{
+   "email":"alice@coder.com",
+   "password":"hunter2",
+   "username":"alice",
+   "name":"alice",
+   "organization_id":"00000000-0000-0000-0000-000000000000",
+   "disable_login":false,
+   "login_type":"none"
+}
+`
+		var req codersdk.CreateUserRequest
+		err := json.Unmarshal([]byte(input), &req)
+		require.NoError(t, err)
+		require.Equal(t, req.Email, "alice@coder.com")
+		require.Equal(t, req.Password, "hunter2")
+		require.Equal(t, req.Username, "alice")
+		require.Equal(t, req.Name, "alice")
+		require.Equal(t, req.OrganizationIDs, []uuid.UUID{uuid.Nil})
+		require.Equal(t, req.UserLoginType, codersdk.LoginTypeNone)
+	})
+
+	t.Run("MultipleOrganizations", func(t *testing.T) {
+		t.Parallel()
+
+		input := `
+{
+   "email":"alice@coder.com",
+   "password":"hunter2",
+   "username":"alice",
+   "name":"alice",
+   "organization_id":"00000000-0000-0000-0000-000000000000",
+   "organization_ids":["a618cb03-99fb-4380-adb6-aa801629a4cf","8309b0dc-44ea-435d-a9ff-72cb302835e4"],
+   "disable_login":false,
+   "login_type":"none"
+}
+`
+		var req codersdk.CreateUserRequest
+		err := json.Unmarshal([]byte(input), &req)
+		require.NoError(t, err)
+		require.Equal(t, req.Email, "alice@coder.com")
+		require.Equal(t, req.Password, "hunter2")
+		require.Equal(t, req.Username, "alice")
+		require.Equal(t, req.Name, "alice")
+		require.ElementsMatch(t, req.OrganizationIDs,
+			[]uuid.UUID{
+				uuid.Nil,
+				uuid.MustParse("a618cb03-99fb-4380-adb6-aa801629a4cf"),
+				uuid.MustParse("8309b0dc-44ea-435d-a9ff-72cb302835e4"),
+			})
+
+		require.Equal(t, req.UserLoginType, codersdk.LoginTypeNone)
+	})
+
+	t.Run("OmittedOrganizations", func(t *testing.T) {
+		t.Parallel()
+
+		input := `
+{
+   "email":"alice@coder.com",
+   "password":"hunter2",
+   "username":"alice",
+   "name":"alice",
+   "disable_login":false,
+   "login_type":"none"
+}
+`
+		var req codersdk.CreateUserRequest
+		err := json.Unmarshal([]byte(input), &req)
+		require.NoError(t, err)
+
+		require.Empty(t, req.OrganizationIDs)
+	})
+}
diff --git a/enterprise/coderd/scim.go b/enterprise/coderd/scim.go
@@ -232,7 +232,7 @@ func (api *API) scimPostUser(rw http.ResponseWriter, r *http.Request) {
 	}
 
 	//nolint:gocritic // needed for SCIM
-	dbUser, _, err = api.AGPL.CreateUser(dbauthz.AsSystemRestricted(ctx), api.Database, agpl.CreateUserRequest{
+	dbUser, err = api.AGPL.CreateUser(dbauthz.AsSystemRestricted(ctx), api.Database, agpl.CreateUserRequest{
 		CreateUserRequest: codersdk.CreateUserRequest{
 			Username:       sUser.UserName,
 			Email:          email,