From 76962273d8ec369538e8282cfa47838196258fc4 Mon Sep 17 00:00:00 2001 From: "J. Scott Miller" Date: Mon, 24 Aug 2026 09:41:21 -0500 Subject: [PATCH] feat: add coder organizations edit for default member roles (#28227) `default_org_member_roles` was only settable through Terraform or a raw `PATCH /api/v2/organizations/{organization}`. This adds a CLI equivalent. ```console coder organizations edit --org acme --default-org-member-roles organization-workspace-access,organization-template-admin coder organizations edit --org acme --default-org-member-roles "" # grant no roles ``` - The flag replaces the organization's current list rather than adding to it. It accepts a comma-separated list, may be repeated, and de-duplicates. An empty value removes every role. - Removing a role prompts for confirmation, listing the roles being removed, unless `-y` is passed. Adding roles applies without a prompt. - The resulting set is printed, so no follow-up command is needed to confirm what was applied. - The organization is chosen with `-O/--org` or `$CODER_ORGANIZATION`, and is implicit when the caller belongs to exactly one organization. The org update endpoint is behind `FeatureMultipleOrganizations`, so the command is enterprise/premium only. Non-built-in roles are rejected by the server, and `dbauthz` already prevents a caller from setting a default they could not assign to a member individually.
Implementation notes ## Shape `codersdk.UpdateOrganizationRequest.DefaultOrgMemberRoles *[]string` and the enterprise `patchOrganization` handler already existed; only the CLI was missing, and nothing in `cli/` called `UpdateOrganization` at all. No server or SDK changes are needed. `organizations edit` follows the partial-update pattern of `coder groups edit`: check `ParsedFlags().Changed(...)`, populate only the fields the user set, and leave the rest nil so the handler preserves them. Passing no flags is an error. Two earlier shapes were tried and dropped: - **A `default-org-member-roles` entry in the `organizations settings` registry.** Every sibling entry unmarshals an SDK type that maps 1:1 to a dedicated endpoint; this field has no such endpoint. Reusing `UpdateOrganizationRequest` for the piped JSON meant a stray `name` field could rename the org, and the documented `show > file`, edit, `set < file` workflow failed on the default organization, whose name is reserved. - **A `--default-org-member-roles` flag on `organizations create`.** `create` cannot set display name, description, or icon either, so a roles-only flag there is asymmetric. It would also require `CreateOrganizationRequest` to grow the field plus server-side validation. ## Known sharp edge `serpent.StringArray` resets its slice when any flag value is empty, so `--default-org-member-roles a --default-org-member-roles ""` clears the list rather than keeping `a`. This is pinned by a test and called out in a code comment. The confirmation prompt catches it interactively; a script passing `-y` would not. A custom flag value that ignores empty occurrences was prototyped and reverted as more machinery than the behavior warranted. ## Validation Manually verified against a local dev deployment with a premium license: setting, adding, clearing, prompt accepted and declined, duplicate and whitespace handling, invalid role rejection, and a plain member being denied. Confirmed with `/authcheck` that a member loses workspace-create access when the roles are cleared and regains it when restored, and that the change is recorded in the audit log. ## Follow-ups worth considering - `--name`, `--display-name`, `--description`, and `--icon` on `organizations edit`; all already exist on `UpdateOrganizationRequest`. Rename deserves its own discussion. - Setting the defaults to `organization-admin` makes every member an org admin, including the ability to edit the org further, and those users still render as plain members in member listings. Reachable today via the API, UI, and Terraform; a warning on privileged roles would help. - `dbauthz.InsertOrganization` has no `canAssignRoles` check, unlike `UpdateOrganization`. Harmless while the only caller passes the deployment default constant. - `docs/admin/users/organizations.md` still points users at the dashboard or a raw `PATCH`.
--- Created by Coder Agents on behalf of @jscottmiller. --- cli/organization.go | 1 + cli/organizationmanage.go | 118 ++++++++ .../coder_organizations_--help.golden | 1 + .../coder_organizations_edit_--help.golden | 31 +++ docs/admin/users/organizations.md | 17 +- docs/manifest.json | 5 + docs/reference/cli/organizations.md | 1 + docs/reference/cli/organizations_edit.md | 54 ++++ enterprise/cli/organization_test.go | 260 ++++++++++++++++++ 9 files changed, 486 insertions(+), 2 deletions(-) create mode 100644 cli/testdata/coder_organizations_edit_--help.golden create mode 100644 docs/reference/cli/organizations_edit.md diff --git a/cli/organization.go b/cli/organization.go index 6ebd28f9ff5..f96c3dda58a 100644 --- a/cli/organization.go +++ b/cli/organization.go @@ -25,6 +25,7 @@ func (r *RootCmd) organizations() *serpent.Command { r.showOrganization(orgContext), r.listOrganizations(), r.createOrganization(), + r.editOrganization(orgContext), r.deleteOrganization(orgContext), r.organizationMembers(orgContext), r.organizationRoles(orgContext), diff --git a/cli/organizationmanage.go b/cli/organizationmanage.go index ce196a1682d..6632532d07c 100644 --- a/cli/organizationmanage.go +++ b/cli/organizationmanage.go @@ -2,12 +2,15 @@ package cli import ( "fmt" + "slices" + "strings" "github.com/google/uuid" "golang.org/x/xerrors" "github.com/coder/coder/v2/cli/cliui" "github.com/coder/coder/v2/codersdk" + "github.com/coder/pretty" "github.com/coder/serpent" ) @@ -56,3 +59,118 @@ func (r *RootCmd) createOrganization() *serpent.Command { return cmd } + +const defaultOrgMemberRolesFlag = "default-org-member-roles" + +func (r *RootCmd) editOrganization(orgContext *OrganizationContext) *serpent.Command { + var defaultOrgMemberRoles []string + cmd := &serpent.Command{ + Use: "edit", + Short: "Edit organization settings.", + Long: FormatExamples( + Example{ + Description: "Replace the roles every member of the organization holds", + Command: "coder organizations edit --default-org-member-roles organization-workspace-access,organization-template-admin", + }, + Example{ + Description: "Grant members no roles at all", + Command: `coder organizations edit --default-org-member-roles ""`, + }, + ), + Middleware: serpent.Chain( + serpent.RequireNArgs(0), + ), + Options: serpent.OptionSet{ + { + Name: defaultOrgMemberRolesFlag, + Flag: defaultOrgMemberRolesFlag, + Description: "Replaces the roles every member of the organization holds. " + + "Accepts a comma-separated list and may be repeated. " + + "An empty value removes every role. " + + "New organizations start with organization-workspace-access, which grants members access to their own workspaces.", + Value: serpent.StringArrayOf(&defaultOrgMemberRoles), + }, + cliui.SkipPromptOption(), + }, + Handler: func(inv *serpent.Invocation) error { + if !inv.ParsedFlags().Changed(defaultOrgMemberRolesFlag) { + return xerrors.Errorf("no changes requested; pass --%s", defaultOrgMemberRolesFlag) + } + + roles, err := parseDefaultOrgMemberRoles(defaultOrgMemberRoles) + if err != nil { + return err + } + + client, err := r.InitClient(inv) + if err != nil { + return err + } + + org, err := orgContext.Selected(inv, client) + if err != nil { + return err + } + + removed := make([]string, 0, len(org.DefaultOrgMemberRoles)) + for _, role := range org.DefaultOrgMemberRoles { + if !slices.Contains(roles, role) { + removed = append(removed, role) + } + } + if len(removed) > 0 { + _, err = cliui.Prompt(inv, cliui.PromptOptions{ + Text: fmt.Sprintf("Remove %s from every member of %s?", + pretty.Sprint(cliui.DefaultStyles.Code, strings.Join(removed, ", ")), + pretty.Sprint(cliui.DefaultStyles.Code, org.Name)), + IsConfirm: true, + Default: cliui.ConfirmNo, + }) + if err != nil { + return err + } + } + + organization, err := client.UpdateOrganization(inv.Context(), org.ID.String(), codersdk.UpdateOrganizationRequest{ + DefaultOrgMemberRoles: &roles, + }) + if err != nil { + return xerrors.Errorf("failed to update organization: %w", err) + } + + _, _ = fmt.Fprintf(inv.Stdout, "Organization %s (%s) updated.\nDefault member roles: %s\n", + organization.Name, organization.ID, formatOrgMemberRoles(organization.DefaultOrgMemberRoles)) + return nil + }, + } + orgContext.AttachOptions(cmd) + + return cmd +} + +// parseDefaultOrgMemberRoles trims and de-duplicates the flag values, +// preserving the order they were given in. An empty set of values means +// "no roles": serpent resets the slice to nil when any flag value is +// empty, so `--flag a --flag ""` also lands here. The returned slice is +// always allocated so the request carries [] rather than null, which the +// server reads as "unspecified". +func parseDefaultOrgMemberRoles(values []string) ([]string, error) { + roles := make([]string, 0, len(values)) + for _, role := range values { + role = strings.TrimSpace(role) + if role == "" { + return nil, xerrors.Errorf("--%s contains an empty role name", defaultOrgMemberRolesFlag) + } + if !slices.Contains(roles, role) { + roles = append(roles, role) + } + } + return roles, nil +} + +func formatOrgMemberRoles(roles []string) string { + if len(roles) == 0 { + return "(none)" + } + return strings.Join(roles, ", ") +} diff --git a/cli/testdata/coder_organizations_--help.golden b/cli/testdata/coder_organizations_--help.golden index 46f5d56a215..2fcdd2ba91d 100644 --- a/cli/testdata/coder_organizations_--help.golden +++ b/cli/testdata/coder_organizations_--help.golden @@ -10,6 +10,7 @@ USAGE: SUBCOMMANDS: create Create a new organization. delete Delete an organization + edit Edit organization settings. list List all organizations members Manage organization members roles Manage organization roles. diff --git a/cli/testdata/coder_organizations_edit_--help.golden b/cli/testdata/coder_organizations_edit_--help.golden new file mode 100644 index 00000000000..8722ce5c3d5 --- /dev/null +++ b/cli/testdata/coder_organizations_edit_--help.golden @@ -0,0 +1,31 @@ +coder v0.0.0-devel + +USAGE: + coder organizations edit [flags] + + Edit organization settings. + + - Replace the roles every member of the organization holds: + + $ coder organizations edit --default-org-member-roles + organization-workspace-access,organization-template-admin + + - Grant members no roles at all: + + $ coder organizations edit --default-org-member-roles "" + +OPTIONS: + -O, --org string, $CODER_ORGANIZATION + Select which organization (uuid or name) to use. + + --default-org-member-roles string-array + Replaces the roles every member of the organization holds. Accepts a + comma-separated list and may be repeated. An empty value removes every + role. New organizations start with organization-workspace-access, + which grants members access to their own workspaces. + + -y, --yes bool + Bypass confirmation prompts. + +——— +Run `coder --help` for a list of global options. diff --git a/docs/admin/users/organizations.md b/docs/admin/users/organizations.md index a5d9fe9cc4c..f5102ff7e1b 100644 --- a/docs/admin/users/organizations.md +++ b/docs/admin/users/organizations.md @@ -139,8 +139,21 @@ role is assigned to them directly, which is useful for restricted accounts that should only hold the minimal member permissions. To edit the default roles in the dashboard, go to -**Admin settings** > **Organizations** > **Roles** > **Default Roles**, or -set `default_org_member_roles` via +**Admin settings** > **Organizations** > **Roles** > **Default Roles**. + +From the CLI, use `coder organizations edit`: + +```shell +coder organizations edit --org \ + --default-org-member-roles organization-workspace-access,organization-template-admin +``` + +The flag replaces the current list rather than adding to it, and accepts a +comma-separated list of built-in role names. Pass an empty value to remove +every role. Removing a role prompts for confirmation unless you pass +`--yes`. + +You can also set `default_org_member_roles` via `PATCH /organizations/{organization}`. ### Limitations diff --git a/docs/manifest.json b/docs/manifest.json index 8f206c65dc9..d2c5c4b23b3 100644 --- a/docs/manifest.json +++ b/docs/manifest.json @@ -1886,6 +1886,11 @@ "description": "Delete an organization", "path": "reference/cli/organizations_delete.md" }, + { + "title": "organizations edit", + "description": "Edit an organization.", + "path": "reference/cli/organizations_edit.md" + }, { "title": "organizations list", "description": "List all organizations", diff --git a/docs/reference/cli/organizations.md b/docs/reference/cli/organizations.md index 02ad6af84ee..f7020cb5a6b 100644 --- a/docs/reference/cli/organizations.md +++ b/docs/reference/cli/organizations.md @@ -27,6 +27,7 @@ coder organizations [flags] [subcommand] | [show](./organizations_show.md) | Show the organization. Using "selected" will show the selected organization from the "--org" flag. Using "me" will show all organizations you are a member of. | | [list](./organizations_list.md) | List all organizations | | [create](./organizations_create.md) | Create a new organization. | +| [edit](./organizations_edit.md) | Edit organization settings. | | [delete](./organizations_delete.md) | Delete an organization | | [members](./organizations_members.md) | Manage organization members | | [roles](./organizations_roles.md) | Manage organization roles. | diff --git a/docs/reference/cli/organizations_edit.md b/docs/reference/cli/organizations_edit.md new file mode 100644 index 00000000000..15cccb253c2 --- /dev/null +++ b/docs/reference/cli/organizations_edit.md @@ -0,0 +1,54 @@ +--- +# Code generated by make gen. DO NOT EDIT. +title: organizations edit +description: Edit organization settings. +--- + + + +Edit organization settings. + +## Usage + +```console +coder organizations edit [flags] +``` + +## Description + +```console + - Replace the roles every member of the organization holds: + + $ coder organizations edit --default-org-member-roles organization-workspace-access,organization-template-admin + + - Grant members no roles at all: + + $ coder organizations edit --default-org-member-roles "" +``` + +## Options + +### --default-org-member-roles + +| | | +|------|---------------------------| +| Type | string-array | + +Replaces the roles every member of the organization holds. Accepts a comma-separated list and may be repeated. An empty value removes every role. New organizations start with organization-workspace-access, which grants members access to their own workspaces. + +### -y, --yes + +| | | +|------|-------------------| +| Type | bool | + +Bypass confirmation prompts. + +### -O, --org + +| | | +|-------------|----------------------------------| +| Type | string | +| Environment | $CODER_ORGANIZATION | + +Select which organization (uuid or name) to use. diff --git a/enterprise/cli/organization_test.go b/enterprise/cli/organization_test.go index 3a7f75350f1..5ff4bb4c94e 100644 --- a/enterprise/cli/organization_test.go +++ b/enterprise/cli/organization_test.go @@ -3,6 +3,7 @@ package cli_test import ( "bytes" "fmt" + "strings" "testing" "github.com/google/uuid" @@ -16,6 +17,7 @@ import ( "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/enterprise/coderd/coderdenttest" "github.com/coder/coder/v2/enterprise/coderd/license" + "github.com/coder/coder/v2/pty/ptytest" "github.com/coder/coder/v2/testutil" "github.com/coder/coder/v2/testutil/expecter" ) @@ -289,3 +291,261 @@ func TestUpdateOrganizationRoles(t *testing.T) { require.ErrorContains(t, err, "The role test-role does not exist.") }) } + +func TestEditOrganization(t *testing.T) { + t.Parallel() + + setup := func(t *testing.T) (*codersdk.Client, codersdk.CreateFirstUserResponse) { + t.Helper() + return coderdenttest.New(t, &coderdenttest.Options{ + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureMultipleOrganizations: 1, + }, + }, + }) + } + + t.Run("SetRoles", func(t *testing.T) { + t.Parallel() + + client, first := setup(t) + ctx := testutil.Context(t, testutil.WaitMedium) + inv, root := clitest.New(t, "organizations", "edit", "-y", + "--default-org-member-roles", codersdk.RoleOrganizationTemplateAdmin+", "+codersdk.RoleOrganizationAuditor, + "--default-org-member-roles", codersdk.RoleOrganizationWorkspaceAccess, + // Duplicates collapse. + "--default-org-member-roles", codersdk.RoleOrganizationAuditor) + //nolint:gocritic // only owners can update orgs + clitest.SetupConfig(t, client, root) + buf := new(bytes.Buffer) + inv.Stdout = buf + + err := inv.WithContext(ctx).Run() + require.NoError(t, err) + require.Contains(t, buf.String(), "Default member roles: "+ + codersdk.RoleOrganizationTemplateAdmin+", "+ + codersdk.RoleOrganizationAuditor+", "+ + codersdk.RoleOrganizationWorkspaceAccess) + + //nolint:gocritic // only owners can read all orgs + org, err := client.Organization(ctx, first.OrganizationID) + require.NoError(t, err) + require.Equal(t, []string{ + codersdk.RoleOrganizationTemplateAdmin, + codersdk.RoleOrganizationAuditor, + codersdk.RoleOrganizationWorkspaceAccess, + }, org.DefaultOrgMemberRoles) + }) + + t.Run("ClearRolesRevokesAccess", func(t *testing.T) { + t.Parallel() + + client, first := setup(t) + ctx := testutil.Context(t, testutil.WaitMedium) + memberClient, _ := coderdtest.CreateAnotherUser(t, client, first.OrganizationID) + + workspaceCheck := codersdk.AuthorizationRequest{Checks: map[string]codersdk.AuthorizationCheck{ + "create": { + Object: codersdk.AuthorizationObject{ + ResourceType: codersdk.ResourceWorkspace, + OrganizationID: first.OrganizationID.String(), + OwnerID: "me", + }, + Action: codersdk.ActionCreate, + }, + }} + before, err := memberClient.AuthCheck(ctx, workspaceCheck) + require.NoError(t, err) + require.True(t, before["create"], "member should start with workspace access") + + inv, root := clitest.New(t, "organizations", "edit", "-y", "--default-org-member-roles", "") + //nolint:gocritic // only owners can update orgs + clitest.SetupConfig(t, client, root) + buf := new(bytes.Buffer) + inv.Stdout = buf + + err = inv.WithContext(ctx).Run() + require.NoError(t, err) + require.Contains(t, buf.String(), "Default member roles: (none)") + + //nolint:gocritic // only owners can read all orgs + org, err := client.Organization(ctx, first.OrganizationID) + require.NoError(t, err) + require.NotNil(t, org.DefaultOrgMemberRoles) + require.Len(t, org.DefaultOrgMemberRoles, 0) + + after, err := memberClient.AuthCheck(ctx, workspaceCheck) + require.NoError(t, err) + require.False(t, after["create"], "member should lose workspace access") + }) + + t.Run("LeavesOtherFields", func(t *testing.T) { + t.Parallel() + + client, first := setup(t) + ctx := testutil.Context(t, testutil.WaitMedium) + //nolint:gocritic // only owners can read all orgs + before, err := client.Organization(ctx, first.OrganizationID) + require.NoError(t, err) + + inv, root := clitest.New(t, "organizations", "edit", "-y", + "--default-org-member-roles", codersdk.RoleOrganizationAuditor) + //nolint:gocritic // only owners can update orgs + clitest.SetupConfig(t, client, root) + inv.Stdout = new(bytes.Buffer) + + err = inv.WithContext(ctx).Run() + require.NoError(t, err) + + //nolint:gocritic // only owners can read all orgs + after, err := client.Organization(ctx, first.OrganizationID) + require.NoError(t, err) + require.Equal(t, []string{codersdk.RoleOrganizationAuditor}, after.DefaultOrgMemberRoles) + require.NotEqual(t, before.DefaultOrgMemberRoles, after.DefaultOrgMemberRoles) + require.Equal(t, before.Name, after.Name) + require.Equal(t, before.DisplayName, after.DisplayName) + require.Equal(t, before.Description, after.Description) + require.Equal(t, before.Icon, after.Icon) + }) + + t.Run("RemovalRequiresConfirmation", func(t *testing.T) { + t.Parallel() + + client, first := setup(t) + ctx := testutil.Context(t, testutil.WaitMedium) + inv, root := clitest.New(t, "organizations", "edit", "--default-org-member-roles", "") + //nolint:gocritic // only owners can update orgs + clitest.SetupConfig(t, client, root) + pty := ptytest.New(t).Attach(inv) + + done := make(chan error, 1) + go func() { + done <- inv.WithContext(ctx).Run() + }() + + pty.ExpectMatch(ctx, codersdk.RoleOrganizationWorkspaceAccess) + pty.WriteLine("no") + require.Error(t, testutil.RequireReceive(ctx, t, done)) + + //nolint:gocritic // only owners can read all orgs + org, err := client.Organization(ctx, first.OrganizationID) + require.NoError(t, err) + require.Equal(t, rbac.DefaultOrgMemberRoles(), org.DefaultOrgMemberRoles) + }) + + t.Run("AddingRolesSkipsConfirmation", func(t *testing.T) { + t.Parallel() + + client, first := setup(t) + ctx := testutil.Context(t, testutil.WaitMedium) + roles := append(rbac.DefaultOrgMemberRoles(), codersdk.RoleOrganizationAuditor) + inv, root := clitest.New(t, "organizations", "edit", + "--default-org-member-roles", strings.Join(roles, ",")) + //nolint:gocritic // only owners can update orgs + clitest.SetupConfig(t, client, root) + inv.Stdout = new(bytes.Buffer) + + err := inv.WithContext(ctx).Run() + require.NoError(t, err) + + //nolint:gocritic // only owners can read all orgs + org, err := client.Organization(ctx, first.OrganizationID) + require.NoError(t, err) + require.Equal(t, roles, org.DefaultOrgMemberRoles) + }) + + t.Run("EmptyValueDiscardsEarlierRoles", func(t *testing.T) { + t.Parallel() + + client, first := setup(t) + ctx := testutil.Context(t, testutil.WaitMedium) + // serpent resets the slice on an empty value, so the earlier role + // is discarded and the list ends up empty. + inv, root := clitest.New(t, "organizations", "edit", "-y", + "--default-org-member-roles", codersdk.RoleOrganizationAuditor, + "--default-org-member-roles", "") + //nolint:gocritic // only owners can update orgs + clitest.SetupConfig(t, client, root) + inv.Stdout = new(bytes.Buffer) + + err := inv.WithContext(ctx).Run() + require.NoError(t, err) + + //nolint:gocritic // only owners can read all orgs + org, err := client.Organization(ctx, first.OrganizationID) + require.NoError(t, err) + require.Empty(t, org.DefaultOrgMemberRoles) + }) + + t.Run("InvalidFlags", func(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + args []string + errorContains string + }{ + { + name: "NoFlags", + args: []string{}, + errorContains: "no changes requested", + }, + { + name: "EmptyRoleName", + args: []string{"--default-org-member-roles", codersdk.RoleOrganizationAuditor + ","}, + errorContains: "empty role name", + }, + { + name: "WhitespaceRoleName", + args: []string{"--default-org-member-roles", " "}, + errorContains: "empty role name", + }, + { + name: "NonBuiltInRole", + args: []string{"--default-org-member-roles", "not-a-built-in-role"}, + errorContains: "Invalid default_org_member_roles entry", + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + client, first := setup(t) + ctx := testutil.Context(t, testutil.WaitMedium) + inv, root := clitest.New(t, append([]string{"organizations", "edit", "-y"}, tc.args...)...) + //nolint:gocritic // only owners can update orgs + clitest.SetupConfig(t, client, root) + inv.Stdout = new(bytes.Buffer) + + err := inv.WithContext(ctx).Run() + require.ErrorContains(t, err, tc.errorContains) + + //nolint:gocritic // only owners can read all orgs + org, err := client.Organization(ctx, first.OrganizationID) + require.NoError(t, err) + require.Equal(t, rbac.DefaultOrgMemberRoles(), org.DefaultOrgMemberRoles) + }) + } + }) + + t.Run("MemberDenied", func(t *testing.T) { + t.Parallel() + + client, first := setup(t) + ctx := testutil.Context(t, testutil.WaitMedium) + memberClient, _ := coderdtest.CreateAnotherUser(t, client, first.OrganizationID) + + inv, root := clitest.New(t, "organizations", "edit", "-y", + "--default-org-member-roles", codersdk.RoleOrganizationAdmin) + clitest.SetupConfig(t, memberClient, root) + inv.Stdout = new(bytes.Buffer) + + err := inv.WithContext(ctx).Run() + require.Error(t, err) + + //nolint:gocritic // only owners can read all orgs + org, err := client.Organization(ctx, first.OrganizationID) + require.NoError(t, err) + require.Equal(t, rbac.DefaultOrgMemberRoles(), org.DefaultOrgMemberRoles) + }) +}