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
This commit is contained in:
Steven Masley
2024-06-20 10:06:37 -05:00
committed by GitHub
parent a1ec8ad6e9
commit a1db6d809e
15 changed files with 321 additions and 0 deletions
+39
View File
@@ -2395,6 +2395,45 @@ const docTemplate = `{
}
}
}
},
"delete": {
"security": [
{
"CoderSessionToken": []
}
],
"produces": [
"application/json"
],
"tags": [
"Members"
],
"summary": "Remove organization member",
"operationId": "remove-organization-member",
"parameters": [
{
"type": "string",
"description": "Organization ID",
"name": "organization",
"in": "path",
"required": true
},
{
"type": "string",
"description": "User ID, name, or me",
"name": "user",
"in": "path",
"required": true
}
],
"responses": {
"200": {
"description": "OK",
"schema": {
"$ref": "#/definitions/codersdk.OrganizationMember"
}
}
}
}
},
"/organizations/{organization}/members/{user}/roles": {
+35
View File
@@ -2097,6 +2097,41 @@
}
}
}
},
"delete": {
"security": [
{
"CoderSessionToken": []
}
],
"produces": ["application/json"],
"tags": ["Members"],
"summary": "Remove organization member",
"operationId": "remove-organization-member",
"parameters": [
{
"type": "string",
"description": "Organization ID",
"name": "organization",
"in": "path",
"required": true
},
{
"type": "string",
"description": "User ID, name, or me",
"name": "user",
"in": "path",
"required": true
}
],
"responses": {
"200": {
"description": "OK",
"schema": {
"$ref": "#/definitions/codersdk.OrganizationMember"
}
}
}
}
},
"/organizations/{organization}/members/{user}/roles": {
+1
View File
@@ -860,6 +860,7 @@ func New(options *Options) *API {
r.Use(
httpmw.ExtractOrganizationMemberParam(options.Database),
)
r.Delete("/", api.deleteOrganizationMember)
r.Put("/roles", api.putMemberRoles)
r.Post("/workspaces", api.postWorkspacesByOrganization)
})
+10
View File
@@ -1035,6 +1035,16 @@ func (q *querier) DeleteOrganization(ctx context.Context, id uuid.UUID) error {
return deleteQ(q.log, q.auth, q.db.GetOrganizationByID, q.db.DeleteOrganization)(ctx, id)
}
func (q *querier) DeleteOrganizationMember(ctx context.Context, arg database.DeleteOrganizationMemberParams) error {
return deleteQ[database.OrganizationMember](q.log, q.auth, func(ctx context.Context, arg database.DeleteOrganizationMemberParams) (database.OrganizationMember, error) {
member, err := database.ExpectOne(q.OrganizationMembers(ctx, database.OrganizationMembersParams(arg)))
if err != nil {
return database.OrganizationMember{}, err
}
return member.OrganizationMember, nil
}, q.db.DeleteOrganizationMember)(ctx, arg)
}
func (q *querier) DeleteReplicasUpdatedBefore(ctx context.Context, updatedAt time.Time) error {
if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceSystem); err != nil {
return err
+17
View File
@@ -628,6 +628,23 @@ func (s *MethodTestSuite) TestOrganization() {
rbac.ResourceAssignOrgRole.InOrg(o.ID), policy.ActionAssign,
rbac.ResourceOrganizationMember.InOrg(o.ID).WithID(u.ID), policy.ActionCreate)
}))
s.Run("DeleteOrganizationMember", s.Subtest(func(db database.Store, check *expects) {
o := dbgen.Organization(s.T(), db, database.Organization{})
u := dbgen.User(s.T(), db, database.User{})
member := dbgen.OrganizationMember(s.T(), db, database.OrganizationMember{UserID: u.ID, OrganizationID: o.ID})
check.Args(database.DeleteOrganizationMemberParams{
OrganizationID: o.ID,
UserID: u.ID,
}).Asserts(
// Reads the org member before it tries to delete it
member, policy.ActionRead,
member, policy.ActionDelete).
// SQL Filter returns a 404
WithNotAuthorized("no rows").
WithCancelled("no rows").
Errors(sql.ErrNoRows)
}))
s.Run("UpdateOrganization", s.Subtest(func(db database.Store, check *expects) {
o := dbgen.Organization(s.T(), db, database.Organization{
Name: "something-unique",
+18
View File
@@ -1632,6 +1632,24 @@ func (q *FakeQuerier) DeleteOrganization(_ context.Context, id uuid.UUID) error
return sql.ErrNoRows
}
func (q *FakeQuerier) DeleteOrganizationMember(_ context.Context, arg database.DeleteOrganizationMemberParams) error {
err := validateDatabaseType(arg)
if err != nil {
return err
}
q.mutex.Lock()
defer q.mutex.Unlock()
deleted := slices.DeleteFunc(q.data.organizationMembers, func(member database.OrganizationMember) bool {
return member.OrganizationID == arg.OrganizationID && member.UserID == arg.UserID
})
if len(deleted) == 0 {
return sql.ErrNoRows
}
return nil
}
func (q *FakeQuerier) DeleteReplicasUpdatedBefore(_ context.Context, before time.Time) error {
q.mutex.Lock()
defer q.mutex.Unlock()
+7
View File
@@ -291,6 +291,13 @@ func (m metricsStore) DeleteOrganization(ctx context.Context, id uuid.UUID) erro
return r0
}
func (m metricsStore) DeleteOrganizationMember(ctx context.Context, arg database.DeleteOrganizationMemberParams) error {
start := time.Now()
r0 := m.s.DeleteOrganizationMember(ctx, arg)
m.queryLatencies.WithLabelValues("DeleteOrganizationMember").Observe(time.Since(start).Seconds())
return r0
}
func (m metricsStore) DeleteReplicasUpdatedBefore(ctx context.Context, updatedAt time.Time) error {
start := time.Now()
err := m.s.DeleteReplicasUpdatedBefore(ctx, updatedAt)
+14
View File
@@ -469,6 +469,20 @@ func (mr *MockStoreMockRecorder) DeleteOrganization(arg0, arg1 any) *gomock.Call
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteOrganization", reflect.TypeOf((*MockStore)(nil).DeleteOrganization), arg0, arg1)
}
// DeleteOrganizationMember mocks base method.
func (m *MockStore) DeleteOrganizationMember(arg0 context.Context, arg1 database.DeleteOrganizationMemberParams) error {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "DeleteOrganizationMember", arg0, arg1)
ret0, _ := ret[0].(error)
return ret0
}
// DeleteOrganizationMember indicates an expected call of DeleteOrganizationMember.
func (mr *MockStoreMockRecorder) DeleteOrganizationMember(arg0, arg1 any) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteOrganizationMember", reflect.TypeOf((*MockStore)(nil).DeleteOrganizationMember), arg0, arg1)
}
// DeleteReplicasUpdatedBefore mocks base method.
func (m *MockStore) DeleteReplicasUpdatedBefore(arg0 context.Context, arg1 time.Time) error {
m.ctrl.T.Helper()
+1
View File
@@ -75,6 +75,7 @@ type sqlcQuerier interface {
DeleteOldWorkspaceAgentLogs(ctx context.Context) error
DeleteOldWorkspaceAgentStats(ctx context.Context) error
DeleteOrganization(ctx context.Context, id uuid.UUID) error
DeleteOrganizationMember(ctx context.Context, arg DeleteOrganizationMemberParams) error
DeleteReplicasUpdatedBefore(ctx context.Context, updatedAt time.Time) error
DeleteTailnetAgent(ctx context.Context, arg DeleteTailnetAgentParams) (DeleteTailnetAgentRow, error)
DeleteTailnetClient(ctx context.Context, arg DeleteTailnetClientParams) (DeleteTailnetClientRow, error)
+19
View File
@@ -3756,6 +3756,25 @@ func (q *sqlQuerier) UpdateOAuth2ProviderAppSecretByID(ctx context.Context, arg
return i, err
}
const deleteOrganizationMember = `-- name: DeleteOrganizationMember :exec
DELETE
FROM
organization_members
WHERE
organization_id = $1 AND
user_id = $2
`
type DeleteOrganizationMemberParams struct {
OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"`
UserID uuid.UUID `db:"user_id" json:"user_id"`
}
func (q *sqlQuerier) DeleteOrganizationMember(ctx context.Context, arg DeleteOrganizationMemberParams) error {
_, err := q.db.ExecContext(ctx, deleteOrganizationMember, arg.OrganizationID, arg.UserID)
return err
}
const getOrganizationIDsByMemberIDs = `-- name: GetOrganizationIDsByMemberIDs :many
SELECT
user_id, array_agg(organization_id) :: uuid [ ] AS "organization_IDs"
@@ -36,6 +36,15 @@ INSERT INTO
VALUES
($1, $2, $3, $4, $5) RETURNING *;
-- name: DeleteOrganizationMember :exec
DELETE
FROM
organization_members
WHERE
organization_id = @organization_id AND
user_id = @user_id
;
-- name: GetOrganizationIDsByMemberIDs :many
SELECT
+32
View File
@@ -62,6 +62,38 @@ func (api *API) postOrganizationMember(rw http.ResponseWriter, r *http.Request)
httpapi.Write(ctx, rw, http.StatusOK, resp[0])
}
// @Summary Remove organization member
// @ID remove-organization-member
// @Security CoderSessionToken
// @Produce json
// @Tags Members
// @Param organization path string true "Organization ID"
// @Param user path string true "User ID, name, or me"
// @Success 200 {object} codersdk.OrganizationMember
// @Router /organizations/{organization}/members/{user} [delete]
func (api *API) deleteOrganizationMember(rw http.ResponseWriter, r *http.Request) {
var (
ctx = r.Context()
organization = httpmw.OrganizationParam(r)
member = httpmw.OrganizationMemberParam(r)
)
err := api.Database.DeleteOrganizationMember(ctx, database.DeleteOrganizationMemberParams{
OrganizationID: organization.ID,
UserID: member.UserID,
})
if httpapi.Is404Error(err) {
httpapi.ResourceNotFound(rw)
return
}
if err != nil {
httpapi.InternalServerError(rw, err)
return
}
httpapi.Write(ctx, rw, http.StatusOK, "organization member removed")
}
// @Summary List organization members
// @ID list-organization-members
// @Security CoderSessionToken
+58
View File
@@ -1,6 +1,7 @@
package coderd_test
import (
"net/http"
"testing"
"github.com/google/uuid"
@@ -114,6 +115,63 @@ func TestListMembers(t *testing.T) {
})
}
func TestRemoveMember(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)
members, err = orgAdminClient.OrganizationMembers(ctx, first.OrganizationID)
require.NoError(t, err)
require.Len(t, members, 2)
require.ElementsMatch(t,
[]uuid.UUID{first.UserID, orgAdmin.ID},
db2sdk.List(members, onlyIDs))
})
t.Run("MemberNotInOrg", func(t *testing.T) {
t.Parallel()
owner := coderdtest.New(t, nil)
first := coderdtest.CreateFirstUser(t, owner)
orgAdminClient, _ := coderdtest.CreateAnotherUser(t, owner, first.OrganizationID, rbac.ScopedRoleOrgAdmin(first.OrganizationID))
ctx := testutil.Context(t, testutil.WaitMedium)
// nolint:gocritic // requires owner to make a new org
org, _ := owner.CreateOrganization(ctx, codersdk.CreateOrganizationRequest{
Name: "other",
DisplayName: "",
Description: "",
Icon: "",
})
_, user := coderdtest.CreateAnotherUser(t, owner, org.ID)
// Delete a user that is not in the organization
err := orgAdminClient.DeleteOrganizationMember(ctx, first.OrganizationID, user.Username)
require.Error(t, err)
var apiError *codersdk.Error
require.ErrorAs(t, err, &apiError)
require.Equal(t, http.StatusNotFound, apiError.StatusCode())
})
}
func onlyIDs(u codersdk.OrganizationMemberWithName) uuid.UUID {
return u.UserID
}
+13
View File
@@ -393,6 +393,19 @@ func (c *Client) PostOrganizationMember(ctx context.Context, organizationID uuid
return member, json.NewDecoder(res.Body).Decode(&member)
}
// DeleteOrganizationMember removes a user from an organization
func (c *Client) DeleteOrganizationMember(ctx context.Context, organizationID uuid.UUID, user string) error {
res, err := c.Request(ctx, http.MethodDelete, fmt.Sprintf("/api/v2/organizations/%s/members/%s", organizationID, user), nil)
if err != nil {
return err
}
defer res.Body.Close()
if res.StatusCode != http.StatusOK {
return ReadBodyAsError(res)
}
return nil
}
// OrganizationMembers lists all members in an organization
func (c *Client) OrganizationMembers(ctx context.Context, organizationID uuid.UUID) ([]OrganizationMemberWithName, error) {
res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/organizations/%s/members/", organizationID), nil)
+48
View File
@@ -363,6 +363,54 @@ curl -X POST http://coder-server:8080/api/v2/organizations/{organization}/member
To perform this operation, you must be authenticated. [Learn more](authentication.md).
## Remove organization member
### Code samples
```shell
# Example request using curl
curl -X DELETE http://coder-server:8080/api/v2/organizations/{organization}/members/{user} \
-H 'Accept: application/json' \
-H 'Coder-Session-Token: API_KEY'
```
`DELETE /organizations/{organization}/members/{user}`
### Parameters
| Name | In | Type | Required | Description |
| -------------- | ---- | ------ | -------- | -------------------- |
| `organization` | path | string | true | Organization ID |
| `user` | path | string | true | User ID, name, or me |
### Example responses
> 200 Response
```json
{
"created_at": "2019-08-24T14:15:22Z",
"organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6",
"roles": [
{
"display_name": "string",
"name": "string",
"organization_id": "string"
}
],
"updated_at": "2019-08-24T14:15:22Z",
"user_id": "a169451c-8525-4352-b8ca-070dd449a1a5"
}
```
### Responses
| Status | Meaning | Description | Schema |
| ------ | ------------------------------------------------------- | ----------- | -------------------------------------------------------------------- |
| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | [codersdk.OrganizationMember](schemas.md#codersdkorganizationmember) |
To perform this operation, you must be authenticated. [Learn more](authentication.md).
## Assign role to organization member
### Code samples