Skip to content

Commit a1db6d8

Browse files
EmyrkEmyrk
authored
chore: implement delete organization member (#13589)
Side effects of removing an organization member will orphan their user resources. These side effects are not addressed here
1 parent a1ec8ad commit a1db6d8

File tree

15 files changed

+321
-0
lines changed

15 files changed

+321
-0
lines changed

‎coderd/apidoc/docs.go

Lines changed: 39 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

Lines changed: 35 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/coderd.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -860,6 +860,7 @@ func New(options *Options) *API {
860860
r.Use(
861861
httpmw.ExtractOrganizationMemberParam(options.Database),
862862
)
863+
r.Delete("/", api.deleteOrganizationMember)
863864
r.Put("/roles", api.putMemberRoles)
864865
r.Post("/workspaces", api.postWorkspacesByOrganization)
865866
})

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1035,6 +1035,16 @@ func (q *querier) DeleteOrganization(ctx context.Context, id uuid.UUID) error {
10351035
return deleteQ(q.log, q.auth, q.db.GetOrganizationByID, q.db.DeleteOrganization)(ctx, id)
10361036
}
10371037

1038+
func (q *querier) DeleteOrganizationMember(ctx context.Context, arg database.DeleteOrganizationMemberParams) error {
1039+
return deleteQ[database.OrganizationMember](q.log, q.auth, func(ctx context.Context, arg database.DeleteOrganizationMemberParams) (database.OrganizationMember, error) {
1040+
member, err := database.ExpectOne(q.OrganizationMembers(ctx, database.OrganizationMembersParams(arg)))
1041+
if err != nil {
1042+
return database.OrganizationMember{}, err
1043+
}
1044+
return member.OrganizationMember, nil
1045+
}, q.db.DeleteOrganizationMember)(ctx, arg)
1046+
}
1047+
10381048
func (q *querier) DeleteReplicasUpdatedBefore(ctx context.Context, updatedAt time.Time) error {
10391049
if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceSystem); err != nil {
10401050
return err

‎coderd/database/dbauthz/dbauthz_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -628,6 +628,23 @@ func (s *MethodTestSuite) TestOrganization() {
628628
rbac.ResourceAssignOrgRole.InOrg(o.ID), policy.ActionAssign,
629629
rbac.ResourceOrganizationMember.InOrg(o.ID).WithID(u.ID), policy.ActionCreate)
630630
}))
631+
s.Run("DeleteOrganizationMember", s.Subtest(func(db database.Store, check *expects) {
632+
o := dbgen.Organization(s.T(), db, database.Organization{})
633+
u := dbgen.User(s.T(), db, database.User{})
634+
member := dbgen.OrganizationMember(s.T(), db, database.OrganizationMember{UserID: u.ID, OrganizationID: o.ID})
635+
636+
check.Args(database.DeleteOrganizationMemberParams{
637+
OrganizationID: o.ID,
638+
UserID: u.ID,
639+
}).Asserts(
640+
// Reads the org member before it tries to delete it
641+
member, policy.ActionRead,
642+
member, policy.ActionDelete).
643+
// SQL Filter returns a 404
644+
WithNotAuthorized("no rows").
645+
WithCancelled("no rows").
646+
Errors(sql.ErrNoRows)
647+
}))
631648
s.Run("UpdateOrganization", s.Subtest(func(db database.Store, check *expects) {
632649
o := dbgen.Organization(s.T(), db, database.Organization{
633650
Name: "something-unique",

‎coderd/database/dbmem/dbmem.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1632,6 +1632,24 @@ func (q *FakeQuerier) DeleteOrganization(_ context.Context, id uuid.UUID) error
16321632
return sql.ErrNoRows
16331633
}
16341634

1635+
func (q *FakeQuerier) DeleteOrganizationMember(_ context.Context, arg database.DeleteOrganizationMemberParams) error {
1636+
err := validateDatabaseType(arg)
1637+
if err != nil {
1638+
return err
1639+
}
1640+
1641+
q.mutex.Lock()
1642+
defer q.mutex.Unlock()
1643+
1644+
deleted := slices.DeleteFunc(q.data.organizationMembers, func(member database.OrganizationMember) bool {
1645+
return member.OrganizationID == arg.OrganizationID && member.UserID == arg.UserID
1646+
})
1647+
if len(deleted) == 0 {
1648+
return sql.ErrNoRows
1649+
}
1650+
return nil
1651+
}
1652+
16351653
func (q *FakeQuerier) DeleteReplicasUpdatedBefore(_ context.Context, before time.Time) error {
16361654
q.mutex.Lock()
16371655
defer q.mutex.Unlock()

‎coderd/database/dbmetrics/dbmetrics.go

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/dbmock/dbmock.go

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/querier.go

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries.sql.go

Lines changed: 19 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/organizationmembers.sql

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,15 @@ INSERT INTO
3636
VALUES
3737
($1, $2, $3, $4, $5) RETURNING *;
3838

39+
-- name: DeleteOrganizationMember :exec
40+
DELETE
41+
FROM
42+
organization_members
43+
WHERE
44+
organization_id = @organization_id AND
45+
user_id = @user_id
46+
;
47+
3948

4049
-- name: GetOrganizationIDsByMemberIDs :many
4150
SELECT

‎coderd/members.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,38 @@ func (api *API) postOrganizationMember(rw http.ResponseWriter, r *http.Request)
6262
httpapi.Write(ctx, rw, http.StatusOK, resp[0])
6363
}
6464

65+
// @Summary Remove organization member
66+
// @ID remove-organization-member
67+
// @Security CoderSessionToken
68+
// @Produce json
69+
// @Tags Members
70+
// @Param organization path string true "Organization ID"
71+
// @Param user path string true "User ID, name, or me"
72+
// @Success 200 {object} codersdk.OrganizationMember
73+
// @Router /organizations/{organization}/members/{user} [delete]
74+
func (api *API) deleteOrganizationMember(rw http.ResponseWriter, r *http.Request) {
75+
var (
76+
ctx = r.Context()
77+
organization = httpmw.OrganizationParam(r)
78+
member = httpmw.OrganizationMemberParam(r)
79+
)
80+
81+
err := api.Database.DeleteOrganizationMember(ctx, database.DeleteOrganizationMemberParams{
82+
OrganizationID: organization.ID,
83+
UserID: member.UserID,
84+
})
85+
if httpapi.Is404Error(err) {
86+
httpapi.ResourceNotFound(rw)
87+
return
88+
}
89+
if err != nil {
90+
httpapi.InternalServerError(rw, err)
91+
return
92+
}
93+
94+
httpapi.Write(ctx, rw, http.StatusOK, "organization member removed")
95+
}
96+
6597
// @Summary List organization members
6698
// @ID list-organization-members
6799
// @Security CoderSessionToken

‎coderd/members_test.go

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package coderd_test
22

33
import (
4+
"net/http"
45
"testing"
56

67
"github.com/google/uuid"
@@ -114,6 +115,63 @@ func TestListMembers(t *testing.T) {
114115
})
115116
}
116117

118+
func TestRemoveMember(t *testing.T) {
119+
t.Parallel()
120+
121+
t.Run("OK", func(t *testing.T) {
122+
t.Parallel()
123+
owner := coderdtest.New(t, nil)
124+
first := coderdtest.CreateFirstUser(t, owner)
125+
orgAdminClient, orgAdmin := coderdtest.CreateAnotherUser(t, owner, first.OrganizationID, rbac.ScopedRoleOrgAdmin(first.OrganizationID))
126+
_, user := coderdtest.CreateAnotherUser(t, owner, first.OrganizationID)
127+
128+
ctx := testutil.Context(t, testutil.WaitMedium)
129+
// Verify the org of 3 members
130+
members, err := orgAdminClient.OrganizationMembers(ctx, first.OrganizationID)
131+
require.NoError(t, err)
132+
require.Len(t, members, 3)
133+
require.ElementsMatch(t,
134+
[]uuid.UUID{first.UserID, user.ID, orgAdmin.ID},
135+
db2sdk.List(members, onlyIDs))
136+
137+
// Delete a member
138+
err = orgAdminClient.DeleteOrganizationMember(ctx, first.OrganizationID, user.Username)
139+
require.NoError(t, err)
140+
141+
members, err = orgAdminClient.OrganizationMembers(ctx, first.OrganizationID)
142+
require.NoError(t, err)
143+
require.Len(t, members, 2)
144+
require.ElementsMatch(t,
145+
[]uuid.UUID{first.UserID, orgAdmin.ID},
146+
db2sdk.List(members, onlyIDs))
147+
})
148+
149+
t.Run("MemberNotInOrg", func(t *testing.T) {
150+
t.Parallel()
151+
owner := coderdtest.New(t, nil)
152+
first := coderdtest.CreateFirstUser(t, owner)
153+
orgAdminClient, _ := coderdtest.CreateAnotherUser(t, owner, first.OrganizationID, rbac.ScopedRoleOrgAdmin(first.OrganizationID))
154+
155+
ctx := testutil.Context(t, testutil.WaitMedium)
156+
// nolint:gocritic // requires owner to make a new org
157+
org, _ := owner.CreateOrganization(ctx, codersdk.CreateOrganizationRequest{
158+
Name: "other",
159+
DisplayName: "",
160+
Description: "",
161+
Icon: "",
162+
})
163+
164+
_, user := coderdtest.CreateAnotherUser(t, owner, org.ID)
165+
166+
// Delete a user that is not in the organization
167+
err := orgAdminClient.DeleteOrganizationMember(ctx, first.OrganizationID, user.Username)
168+
require.Error(t, err)
169+
var apiError *codersdk.Error
170+
require.ErrorAs(t, err, &apiError)
171+
require.Equal(t, http.StatusNotFound, apiError.StatusCode())
172+
})
173+
}
174+
117175
func onlyIDs(u codersdk.OrganizationMemberWithName) uuid.UUID {
118176
return u.UserID
119177
}

‎codersdk/users.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,19 @@ func (c *Client) PostOrganizationMember(ctx context.Context, organizationID uuid
393393
return member, json.NewDecoder(res.Body).Decode(&member)
394394
}
395395

396+
// DeleteOrganizationMember removes a user from an organization
397+
func (c *Client) DeleteOrganizationMember(ctx context.Context, organizationID uuid.UUID, user string) error {
398+
res, err := c.Request(ctx, http.MethodDelete, fmt.Sprintf("/api/v2/organizations/%s/members/%s", organizationID, user), nil)
399+
if err != nil {
400+
return err
401+
}
402+
defer res.Body.Close()
403+
if res.StatusCode != http.StatusOK {
404+
return ReadBodyAsError(res)
405+
}
406+
return nil
407+
}
408+
396409
// OrganizationMembers lists all members in an organization
397410
func (c *Client) OrganizationMembers(ctx context.Context, organizationID uuid.UUID) ([]OrganizationMemberWithName, error) {
398411
res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/organizations/%s/members/", organizationID), nil)

0 commit comments

Comments
 (0)