coder · Emyrk · Aug 7, 2024 · Aug 1, 2024 · Aug 6, 2024 · Aug 6, 2024
diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go
diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json
diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go
@@ -958,6 +958,20 @@ func (q *querier) DeleteCoordinator(ctx context.Context, id uuid.UUID) error {
 	return q.db.DeleteCoordinator(ctx, id)
 }
 
+func (q *querier) DeleteCustomRole(ctx context.Context, arg database.DeleteCustomRoleParams) error {
+	if arg.OrganizationID.UUID != uuid.Nil {
+		if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceAssignOrgRole.InOrg(arg.OrganizationID.UUID)); err != nil {
+			return err
+		}
+	} else {
+		if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceAssignRole); err != nil {
+			return err
+		}
+	}
+
+	return q.db.DeleteCustomRole(ctx, arg)
+}
+
 func (q *querier) DeleteExternalAuthLink(ctx context.Context, arg database.DeleteExternalAuthLinkParams) error {
 	return fetchAndExec(q.log, q.auth, policy.ActionUpdatePersonal, func(ctx context.Context, arg database.DeleteExternalAuthLinkParams) (database.ExternalAuthLink, error) {
 		//nolint:gosimple

diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go
@@ -1247,6 +1247,31 @@ func (s *MethodTestSuite) TestUser() {
 	s.Run("CustomRoles", s.Subtest(func(db database.Store, check *expects) {
 		check.Args(database.CustomRolesParams{}).Asserts(rbac.ResourceAssignRole, policy.ActionRead).Returns([]database.CustomRole{})
 	}))
+	s.Run("Organization/DeleteCustomRole", s.Subtest(func(db database.Store, check *expects) {
+		customRole := dbgen.CustomRole(s.T(), db, database.CustomRole{
+			OrganizationID: uuid.NullUUID{
+				UUID:  uuid.New(),
+				Valid: true,
+			},
+		})
+		check.Args(database.DeleteCustomRoleParams{
+			Name:           customRole.Name,
+			OrganizationID: customRole.OrganizationID,
+		}).Asserts(
+			rbac.ResourceAssignOrgRole.InOrg(customRole.OrganizationID.UUID), policy.ActionDelete)
+	}))
+	s.Run("Site/DeleteCustomRole", s.Subtest(func(db database.Store, check *expects) {
+		customRole := dbgen.CustomRole(s.T(), db, database.CustomRole{
+			OrganizationID: uuid.NullUUID{
+				UUID:  uuid.Nil,
+				Valid: false,
+			},
+		})
+		check.Args(database.DeleteCustomRoleParams{
+			Name: customRole.Name,
+		}).Asserts(
+			rbac.ResourceAssignRole, policy.ActionDelete)
+	}))
 	s.Run("Blank/UpsertCustomRole", s.Subtest(func(db database.Store, check *expects) {
 		// Blank is no perms in the role
 		check.Args(database.UpsertCustomRoleParams{

diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go
@@ -1381,6 +1381,35 @@ func (*FakeQuerier) DeleteCoordinator(context.Context, uuid.UUID) error {
 	return ErrUnimplemented
 }
 
+func (q *FakeQuerier) DeleteCustomRole(_ context.Context, arg database.DeleteCustomRoleParams) error {
+	err := validateDatabaseType(arg)
+	if err != nil {
+		return err
+	}
+
+	q.mutex.RLock()
+	defer q.mutex.RUnlock()
+
+	initial := len(q.data.customRoles)
+	q.data.customRoles = slices.DeleteFunc(q.data.customRoles, func(role database.CustomRole) bool {
+		return role.OrganizationID.UUID == arg.OrganizationID.UUID && role.Name == arg.Name
+	})
+	if initial == len(q.data.customRoles) {
+		return sql.ErrNoRows
+	}
+
+	// Emulate the trigger 'remove_organization_member_custom_role'
+	for i, mem := range q.organizationMembers {
+		if mem.OrganizationID == arg.OrganizationID.UUID {
+			mem.Roles = slices.DeleteFunc(mem.Roles, func(role string) bool {
+				return role == arg.Name
+			})
+			q.organizationMembers[i] = mem
+		}
+	}
+	return nil
+}
+
 func (q *FakeQuerier) DeleteExternalAuthLink(_ context.Context, arg database.DeleteExternalAuthLinkParams) error {
 	err := validateDatabaseType(arg)
 	if err != nil {

diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go
diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go
diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql
diff --git a/coderd/database/migrations/000241_delete_user_roles.down.sql b/coderd/database/migrations/000241_delete_user_roles.down.sql
@@ -0,0 +1,2 @@
+DROP TRIGGER IF EXISTS remove_organization_member_custom_role ON custom_roles;
+DROP FUNCTION IF EXISTS remove_organization_member_role;
diff --git a/coderd/database/migrations/000241_delete_user_roles.up.sql b/coderd/database/migrations/000241_delete_user_roles.up.sql
@@ -0,0 +1,35 @@
+-- When a custom role is deleted, we need to remove the assigned role
+-- from all organization members that have it.
+-- This action cannot be reverted, so deleting a custom role should be
+-- done with caution.
+CREATE OR REPLACE FUNCTION remove_organization_member_role()
+	RETURNS TRIGGER AS
+$$
+BEGIN
+	-- Delete the role from all organization members that have it.
+	-- TODO: When site wide custom roles are supported, if the
+	--	organization_id is null, we should remove the role from the 'users'
+	--	table instead.
+	IF OLD.organization_id IS NOT NULL THEN
+		UPDATE organization_members
+		-- this is a noop if the role is not assigned to the member
+		SET roles = array_remove(roles, OLD.name)
+		WHERE
+			-- Scope to the correct organization
+			organization_members.organization_id = OLD.organization_id;
+	END IF;
+	RETURN OLD;
+END;
+$$ LANGUAGE plpgsql;
+
+
+-- Attach the function to deleting the custom role
+CREATE TRIGGER remove_organization_member_custom_role
+	BEFORE DELETE ON custom_roles FOR EACH ROW
+	EXECUTE PROCEDURE remove_organization_member_role();
+
+
+COMMENT ON TRIGGER
+	remove_organization_member_custom_role
+	ON custom_roles IS
+		'When a custom_role is deleted, this trigger removes the role from all organization members.';
diff --git a/coderd/database/querier.go b/coderd/database/querier.go
diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go
diff --git a/coderd/database/queries/roles.sql b/coderd/database/queries/roles.sql
@@ -25,6 +25,13 @@ WHERE
   END
 ;
 
+-- name: DeleteCustomRole :exec
+DELETE FROM
+	custom_roles
+WHERE
+	name = lower(@name)
+	AND organization_id = @organization_id
+;
 
 -- name: UpsertCustomRole :one
 INSERT INTO

diff --git a/coderd/httpapi/httpapi.go b/coderd/httpapi/httpapi.go
@@ -106,12 +106,24 @@ func Is404Error(err error) bool {
 		return false
 	}
 
+	// This tests for dbauthz.IsNotAuthorizedError and rbac.IsUnauthorizedError.
+	if IsUnauthorizedError(err) {
+		return true
+	}
+	return xerrors.Is(err, sql.ErrNoRows)
+}
+
+func IsUnauthorizedError(err error) bool {
+	if err == nil {
+		return false
+	}
+
 	// This tests for dbauthz.IsNotAuthorizedError and rbac.IsUnauthorizedError.
 	var unauthorized httpapiconstraints.IsUnauthorizedError
 	if errors.As(err, &unauthorized) && unauthorized.IsUnauthorized() {
 		return true
 	}
-	return xerrors.Is(err, sql.ErrNoRows)
+	return false
 }
 
 // Convenience error functions don't take contexts since their responses are

diff --git a/codersdk/roles.go b/codersdk/roles.go
@@ -105,6 +105,20 @@ func (c *Client) PatchOrganizationRole(ctx context.Context, role Role) (Role, er
 	return r, json.NewDecoder(res.Body).Decode(&r)
 }
 
+// DeleteOrganizationRole will delete a custom organization role
+func (c *Client) DeleteOrganizationRole(ctx context.Context, organizationID uuid.UUID, roleName string) error {
+	res, err := c.Request(ctx, http.MethodDelete,
+		fmt.Sprintf("/api/v2/organizations/%s/members/roles/%s", organizationID.String(), roleName), nil)
+	if err != nil {
+		return err
+	}
+	defer res.Body.Close()
+	if res.StatusCode != http.StatusNoContent {
+		return ReadBodyAsError(res)
+	}
+	return nil
+}
+
 // ListSiteRoles lists all assignable site wide roles.
 func (c *Client) ListSiteRoles(ctx context.Context) ([]AssignableRoles, error) {
 	res, err := c.Request(ctx, http.MethodGet, "/api/v2/users/roles", nil)
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		DROP TRIGGER IF EXISTS remove_organization_member_custom_role ON custom_roles;
		DROP FUNCTION IF EXISTS remove_organization_member_role;