Skip to content

Commit 2c13797

Browse files
EmyrkEmyrk
authored
chore: implement deleting custom roles (#14101)
* chore: implement deleting custom roles * add trigger to delete role from organization members on delete * chore: add comments to explain populated field
1 parent d0feb70 commit 2c13797

File tree

19 files changed

+627
-2
lines changed

19 files changed

+627
-2
lines changed

‎coderd/apidoc/docs.go

+45
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

+41
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/dbauthz/dbauthz.go

+14
Original file line numberDiff line numberDiff line change
@@ -958,6 +958,20 @@ func (q *querier) DeleteCoordinator(ctx context.Context, id uuid.UUID) error {
958958
return q.db.DeleteCoordinator(ctx, id)
959959
}
960960

961+
func (q *querier) DeleteCustomRole(ctx context.Context, arg database.DeleteCustomRoleParams) error {
962+
if arg.OrganizationID.UUID != uuid.Nil {
963+
if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceAssignOrgRole.InOrg(arg.OrganizationID.UUID)); err != nil {
964+
return err
965+
}
966+
} else {
967+
if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceAssignRole); err != nil {
968+
return err
969+
}
970+
}
971+
972+
return q.db.DeleteCustomRole(ctx, arg)
973+
}
974+
961975
func (q *querier) DeleteExternalAuthLink(ctx context.Context, arg database.DeleteExternalAuthLinkParams) error {
962976
return fetchAndExec(q.log, q.auth, policy.ActionUpdatePersonal, func(ctx context.Context, arg database.DeleteExternalAuthLinkParams) (database.ExternalAuthLink, error) {
963977
//nolint:gosimple

‎coderd/database/dbauthz/dbauthz_test.go

+25
Original file line numberDiff line numberDiff line change
@@ -1247,6 +1247,31 @@ func (s *MethodTestSuite) TestUser() {
12471247
s.Run("CustomRoles", s.Subtest(func(db database.Store, check *expects) {
12481248
check.Args(database.CustomRolesParams{}).Asserts(rbac.ResourceAssignRole, policy.ActionRead).Returns([]database.CustomRole{})
12491249
}))
1250+
s.Run("Organization/DeleteCustomRole", s.Subtest(func(db database.Store, check *expects) {
1251+
customRole := dbgen.CustomRole(s.T(), db, database.CustomRole{
1252+
OrganizationID: uuid.NullUUID{
1253+
UUID: uuid.New(),
1254+
Valid: true,
1255+
},
1256+
})
1257+
check.Args(database.DeleteCustomRoleParams{
1258+
Name: customRole.Name,
1259+
OrganizationID: customRole.OrganizationID,
1260+
}).Asserts(
1261+
rbac.ResourceAssignOrgRole.InOrg(customRole.OrganizationID.UUID), policy.ActionDelete)
1262+
}))
1263+
s.Run("Site/DeleteCustomRole", s.Subtest(func(db database.Store, check *expects) {
1264+
customRole := dbgen.CustomRole(s.T(), db, database.CustomRole{
1265+
OrganizationID: uuid.NullUUID{
1266+
UUID: uuid.Nil,
1267+
Valid: false,
1268+
},
1269+
})
1270+
check.Args(database.DeleteCustomRoleParams{
1271+
Name: customRole.Name,
1272+
}).Asserts(
1273+
rbac.ResourceAssignRole, policy.ActionDelete)
1274+
}))
12501275
s.Run("Blank/UpsertCustomRole", s.Subtest(func(db database.Store, check *expects) {
12511276
// Blank is no perms in the role
12521277
check.Args(database.UpsertCustomRoleParams{

‎coderd/database/dbmem/dbmem.go

+29
Original file line numberDiff line numberDiff line change
@@ -1381,6 +1381,35 @@ func (*FakeQuerier) DeleteCoordinator(context.Context, uuid.UUID) error {
13811381
return ErrUnimplemented
13821382
}
13831383

1384+
func (q *FakeQuerier) DeleteCustomRole(_ context.Context, arg database.DeleteCustomRoleParams) error {
1385+
err := validateDatabaseType(arg)
1386+
if err != nil {
1387+
return err
1388+
}
1389+
1390+
q.mutex.RLock()
1391+
defer q.mutex.RUnlock()
1392+
1393+
initial := len(q.data.customRoles)
1394+
q.data.customRoles = slices.DeleteFunc(q.data.customRoles, func(role database.CustomRole) bool {
1395+
return role.OrganizationID.UUID == arg.OrganizationID.UUID && role.Name == arg.Name
1396+
})
1397+
if initial == len(q.data.customRoles) {
1398+
return sql.ErrNoRows
1399+
}
1400+
1401+
// Emulate the trigger 'remove_organization_member_custom_role'
1402+
for i, mem := range q.organizationMembers {
1403+
if mem.OrganizationID == arg.OrganizationID.UUID {
1404+
mem.Roles = slices.DeleteFunc(mem.Roles, func(role string) bool {
1405+
return role == arg.Name
1406+
})
1407+
q.organizationMembers[i] = mem
1408+
}
1409+
}
1410+
return nil
1411+
}
1412+
13841413
func (q *FakeQuerier) DeleteExternalAuthLink(_ context.Context, arg database.DeleteExternalAuthLinkParams) error {
13851414
err := validateDatabaseType(arg)
13861415
if err != nil {

‎coderd/database/dbmetrics/dbmetrics.go

+7
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/dbmock/dbmock.go

+14
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/dump.sql

+24
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/migrations/000241_delete_user_roles.down.sql

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
DROP TRIGGER IF EXISTS remove_organization_member_custom_role ON custom_roles;
2+
DROP FUNCTION IF EXISTS remove_organization_member_role;

‎coderd/database/migrations/000241_delete_user_roles.up.sql

+35
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
-- When a custom role is deleted, we need to remove the assigned role
2+
-- from all organization members that have it.
3+
-- This action cannot be reverted, so deleting a custom role should be
4+
-- done with caution.
5+
CREATE OR REPLACE FUNCTION remove_organization_member_role()
6+
RETURNS TRIGGER AS
7+
$$
8+
BEGIN
9+
-- Delete the role from all organization members that have it.
10+
-- TODO: When site wide custom roles are supported, if the
11+
-- organization_id is null, we should remove the role from the 'users'
12+
-- table instead.
13+
IF OLD.organization_id IS NOT NULL THEN
14+
UPDATE organization_members
15+
-- this is a noop if the role is not assigned to the member
16+
SET roles = array_remove(roles, OLD.name)
17+
WHERE
18+
-- Scope to the correct organization
19+
organization_members.organization_id = OLD.organization_id;
20+
END IF;
21+
RETURN OLD;
22+
END;
23+
$$ LANGUAGE plpgsql;
24+
25+
26+
-- Attach the function to deleting the custom role
27+
CREATE TRIGGER remove_organization_member_custom_role
28+
BEFORE DELETE ON custom_roles FOR EACH ROW
29+
EXECUTE PROCEDURE remove_organization_member_role();
30+
31+
32+
COMMENT ON TRIGGER
33+
remove_organization_member_custom_role
34+
ON custom_roles IS
35+
'When a custom_role is deleted, this trigger removes the role from all organization members.';

‎coderd/database/querier.go

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries.sql.go

+18
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/roles.sql

+7
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,13 @@ WHERE
2525
END
2626
;
2727

28+
-- name: DeleteCustomRole :exec
29+
DELETE FROM
30+
custom_roles
31+
WHERE
32+
name = lower(@name)
33+
AND organization_id = @organization_id
34+
;
2835

2936
-- name: UpsertCustomRole :one
3037
INSERT INTO

‎coderd/httpapi/httpapi.go

+13-1
Original file line numberDiff line numberDiff line change
@@ -106,12 +106,24 @@ func Is404Error(err error) bool {
106106
return false
107107
}
108108

109+
// This tests for dbauthz.IsNotAuthorizedError and rbac.IsUnauthorizedError.
110+
if IsUnauthorizedError(err) {
111+
return true
112+
}
113+
return xerrors.Is(err, sql.ErrNoRows)
114+
}
115+
116+
func IsUnauthorizedError(err error) bool {
117+
if err == nil {
118+
return false
119+
}
120+
109121
// This tests for dbauthz.IsNotAuthorizedError and rbac.IsUnauthorizedError.
110122
var unauthorized httpapiconstraints.IsUnauthorizedError
111123
if errors.As(err, &unauthorized) && unauthorized.IsUnauthorized() {
112124
return true
113125
}
114-
return xerrors.Is(err, sql.ErrNoRows)
126+
return false
115127
}
116128

117129
// Convenience error functions don't take contexts since their responses are

‎codersdk/roles.go

+14
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,20 @@ func (c *Client) PatchOrganizationRole(ctx context.Context, role Role) (Role, er
105105
return r, json.NewDecoder(res.Body).Decode(&r)
106106
}
107107

108+
// DeleteOrganizationRole will delete a custom organization role
109+
func (c *Client) DeleteOrganizationRole(ctx context.Context, organizationID uuid.UUID, roleName string) error {
110+
res, err := c.Request(ctx, http.MethodDelete,
111+
fmt.Sprintf("/api/v2/organizations/%s/members/roles/%s", organizationID.String(), roleName), nil)
112+
if err != nil {
113+
return err
114+
}
115+
defer res.Body.Close()
116+
if res.StatusCode != http.StatusNoContent {
117+
return ReadBodyAsError(res)
118+
}
119+
return nil
120+
}
121+
108122
// ListSiteRoles lists all assignable site wide roles.
109123
func (c *Client) ListSiteRoles(ctx context.Context) ([]AssignableRoles, error) {
110124
res, err := c.Request(ctx, http.MethodGet, "/api/v2/users/roles", nil)

0 commit comments

Comments
 (0)