diff --git a/cli/organizationmembers_test.go b/cli/organizationmembers_test.go index e17b268ea798a..6cd8b9d3ccd4a 100644 --- a/cli/organizationmembers_test.go +++ b/cli/organizationmembers_test.go @@ -34,49 +34,3 @@ func TestListOrganizationMembers(t *testing.T) { require.Contains(t, buf.String(), owner.UserID.String()) }) } - -func TestRemoveOrganizationMembers(t *testing.T) { - t.Parallel() - - t.Run("OK", func(t *testing.T) { - t.Parallel() - - ownerClient := coderdtest.New(t, &coderdtest.Options{}) - owner := coderdtest.CreateFirstUser(t, ownerClient) - orgAdminClient, _ := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.ScopedRoleOrgAdmin(owner.OrganizationID)) - _, user := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID) - - ctx := testutil.Context(t, testutil.WaitMedium) - - inv, root := clitest.New(t, "organization", "members", "remove", "-O", owner.OrganizationID.String(), user.Username) - clitest.SetupConfig(t, orgAdminClient, root) - - buf := new(bytes.Buffer) - inv.Stdout = buf - err := inv.WithContext(ctx).Run() - require.NoError(t, err) - - members, err := orgAdminClient.OrganizationMembers(ctx, owner.OrganizationID) - require.NoError(t, err) - - require.Len(t, members, 2) - }) - - t.Run("UserNotExists", func(t *testing.T) { - t.Parallel() - - ownerClient := coderdtest.New(t, &coderdtest.Options{}) - owner := coderdtest.CreateFirstUser(t, ownerClient) - orgAdminClient, _ := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.ScopedRoleOrgAdmin(owner.OrganizationID)) - - ctx := testutil.Context(t, testutil.WaitMedium) - - inv, root := clitest.New(t, "organization", "members", "remove", "-O", owner.OrganizationID.String(), "random_name") - clitest.SetupConfig(t, orgAdminClient, root) - - buf := new(bytes.Buffer) - inv.Stdout = buf - err := inv.WithContext(ctx).Run() - require.ErrorContains(t, err, "must be an existing uuid or username") - }) -} diff --git a/coderd/members.go b/coderd/members.go index 4c28d4b6434f6..805bdafbd0447 100644 --- a/coderd/members.go +++ b/coderd/members.go @@ -106,6 +106,19 @@ func (api *API) deleteOrganizationMember(rw http.ResponseWriter, r *http.Request aReq.Old = member.OrganizationMember.Auditable(member.Username) defer commitAudit() + if organization.IsDefault { + // Multi-organizations is currently an experiment, which means it is feasible + // for a deployment to enable, then disable this. To maintain backwards + // compatibility, this safety is necessary. + // TODO: Remove this check when multi-organizations is fully supported. + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Removing members from the default organization is not supported.", + Detail: "Multi-organizations is currently an experiment, and until it is fully supported, the default org should be protected.", + Validations: nil, + }) + return + } + if member.UserID == apiKey.UserID { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{Message: "cannot remove self from an organization"}) return diff --git a/coderd/members_test.go b/coderd/members_test.go index 8ca655590c956..13b6779c30663 100644 --- a/coderd/members_test.go +++ b/coderd/members_test.go @@ -30,54 +30,41 @@ func TestAddMember(t *testing.T) { }) } -func TestListMembers(t *testing.T) { +func TestDeleteMember(t *testing.T) { t.Parallel() - t.Run("OK", func(t *testing.T) { + t.Run("NotAllowed", func(t *testing.T) { t.Parallel() owner := coderdtest.New(t, nil) first := coderdtest.CreateFirstUser(t, owner) + _, user := coderdtest.CreateAnotherUser(t, owner, first.OrganizationID) - client, user := coderdtest.CreateAnotherUser(t, owner, first.OrganizationID, rbac.ScopedRoleOrgAdmin(first.OrganizationID)) - - ctx := testutil.Context(t, testutil.WaitShort) - members, err := client.OrganizationMembers(ctx, first.OrganizationID) - require.NoError(t, err) - require.Len(t, members, 2) - require.ElementsMatch(t, - []uuid.UUID{first.UserID, user.ID}, - db2sdk.List(members, onlyIDs)) + ctx := testutil.Context(t, testutil.WaitMedium) + // Deleting members from the default org is not allowed. + // If this behavior changes, and we allow deleting members from the default org, + // this test should be updated to check there is no error. + // nolint:gocritic // must be an owner to see the user + err := owner.DeleteOrganizationMember(ctx, first.OrganizationID, user.Username) + require.ErrorContains(t, err, "Multi-organizations is currently an experiment") }) } -func TestRemoveMember(t *testing.T) { +func TestListMembers(t *testing.T) { t.Parallel() t.Run("OK", func(t *testing.T) { t.Parallel() owner := coderdtest.New(t, nil) first := coderdtest.CreateFirstUser(t, owner) - orgAdminClient, orgAdmin := coderdtest.CreateAnotherUser(t, owner, first.OrganizationID, rbac.ScopedRoleOrgAdmin(first.OrganizationID)) - _, user := coderdtest.CreateAnotherUser(t, owner, first.OrganizationID) - ctx := testutil.Context(t, testutil.WaitMedium) - // Verify the org of 3 members - members, err := orgAdminClient.OrganizationMembers(ctx, first.OrganizationID) - require.NoError(t, err) - require.Len(t, members, 3) - require.ElementsMatch(t, - []uuid.UUID{first.UserID, user.ID, orgAdmin.ID}, - db2sdk.List(members, onlyIDs)) - - // Delete a member - err = orgAdminClient.DeleteOrganizationMember(ctx, first.OrganizationID, user.Username) - require.NoError(t, err) + client, user := coderdtest.CreateAnotherUser(t, owner, first.OrganizationID, rbac.ScopedRoleOrgAdmin(first.OrganizationID)) - members, err = orgAdminClient.OrganizationMembers(ctx, first.OrganizationID) + ctx := testutil.Context(t, testutil.WaitShort) + members, err := client.OrganizationMembers(ctx, first.OrganizationID) require.NoError(t, err) require.Len(t, members, 2) require.ElementsMatch(t, - []uuid.UUID{first.UserID, orgAdmin.ID}, + []uuid.UUID{first.UserID, user.ID}, db2sdk.List(members, onlyIDs)) }) } diff --git a/enterprise/cli/organizationmembers_test.go b/enterprise/cli/organizationmembers_test.go index fb6a7cb286058..b33d597faacae 100644 --- a/enterprise/cli/organizationmembers_test.go +++ b/enterprise/cli/organizationmembers_test.go @@ -15,6 +15,64 @@ import ( "github.com/coder/coder/v2/testutil" ) +func TestRemoveOrganizationMembers(t *testing.T) { + t.Parallel() + + t.Run("OK", func(t *testing.T) { + t.Parallel() + dv := coderdtest.DeploymentValues(t) + dv.Experiments = []string{string(codersdk.ExperimentMultiOrganization)} + + ownerClient, _ := coderdenttest.New(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + DeploymentValues: dv, + }, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureMultipleOrganizations: 1, + }, + }, + }) + + secondOrganization := coderdenttest.CreateOrganization(t, ownerClient, coderdenttest.CreateOrganizationOptions{}) + orgAdminClient, _ := coderdtest.CreateAnotherUser(t, ownerClient, secondOrganization.ID, rbac.ScopedRoleOrgAdmin(secondOrganization.ID)) + _, user := coderdtest.CreateAnotherUser(t, ownerClient, secondOrganization.ID) + + ctx := testutil.Context(t, testutil.WaitMedium) + + inv, root := clitest.New(t, "organization", "members", "remove", "-O", secondOrganization.ID.String(), user.Username) + clitest.SetupConfig(t, orgAdminClient, root) + + buf := new(bytes.Buffer) + inv.Stdout = buf + err := inv.WithContext(ctx).Run() + require.NoError(t, err) + + members, err := orgAdminClient.OrganizationMembers(ctx, secondOrganization.ID) + require.NoError(t, err) + + require.Len(t, members, 2) + }) + + t.Run("UserNotExists", func(t *testing.T) { + t.Parallel() + + ownerClient := coderdtest.New(t, &coderdtest.Options{}) + owner := coderdtest.CreateFirstUser(t, ownerClient) + orgAdminClient, _ := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.ScopedRoleOrgAdmin(owner.OrganizationID)) + + ctx := testutil.Context(t, testutil.WaitMedium) + + inv, root := clitest.New(t, "organization", "members", "remove", "-O", owner.OrganizationID.String(), "random_name") + clitest.SetupConfig(t, orgAdminClient, root) + + buf := new(bytes.Buffer) + inv.Stdout = buf + err := inv.WithContext(ctx).Run() + require.ErrorContains(t, err, "must be an existing uuid or username") + }) +} + func TestEnterpriseListOrganizationMembers(t *testing.T) { t.Parallel() diff --git a/enterprise/members_test.go b/enterprise/members_test.go index c328ce71d05fd..e29912be1bda2 100644 --- a/enterprise/members_test.go +++ b/enterprise/members_test.go @@ -18,6 +18,47 @@ import ( func TestEnterpriseMembers(t *testing.T) { t.Parallel() + t.Run("Remove", func(t *testing.T) { + t.Parallel() + dv := coderdtest.DeploymentValues(t) + dv.Experiments = []string{string(codersdk.ExperimentMultiOrganization)} + owner, first := coderdenttest.New(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + DeploymentValues: dv, + }, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureMultipleOrganizations: 1, + }, + }, + }) + + secondOrg := coderdenttest.CreateOrganization(t, owner, coderdenttest.CreateOrganizationOptions{}) + + orgAdminClient, orgAdmin := coderdtest.CreateAnotherUser(t, owner, secondOrg.ID, rbac.ScopedRoleOrgAdmin(secondOrg.ID)) + _, user := coderdtest.CreateAnotherUser(t, owner, secondOrg.ID) + + ctx := testutil.Context(t, testutil.WaitMedium) + // Verify the org of 3 members + members, err := orgAdminClient.OrganizationMembers(ctx, secondOrg.ID) + require.NoError(t, err) + require.Len(t, members, 3) + require.ElementsMatch(t, + []uuid.UUID{first.UserID, user.ID, orgAdmin.ID}, + db2sdk.List(members, onlyIDs)) + + // Delete a member + err = orgAdminClient.DeleteOrganizationMember(ctx, secondOrg.ID, user.Username) + require.NoError(t, err) + + members, err = orgAdminClient.OrganizationMembers(ctx, secondOrg.ID) + require.NoError(t, err) + require.Len(t, members, 2) + require.ElementsMatch(t, + []uuid.UUID{first.UserID, orgAdmin.ID}, + db2sdk.List(members, onlyIDs)) + }) + t.Run("PostUser", func(t *testing.T) { t.Parallel()