mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
feat: add organization scoping to chats (#23827)
Fixes https://github.com/coder/internal/issues/1436 * Adds organization_id to chats with backfill (workspace org → user org membership → default org) * No support yet for ACLs (follow-up issue) - Cross-org workspace binding rejected (both in `CreateChatRequest` and in `create_workspace` tool - Adds `OrganizationAutocomplete` to `AgentCreateForm` - Docs updated with `organization_id` in chats-api.md > 🤖 Written by a Coder Agent. Reviewed by many humans and many agents. --------- Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
This commit is contained in:
co-authored by
Mathias Fredriksson
parent
cb0b84a2d3
commit
22062ec52e
@@ -62,6 +62,7 @@ type AgentConnFunc func(
|
||||
type CreateWorkspaceOptions struct {
|
||||
DB database.Store
|
||||
OwnerID uuid.UUID
|
||||
OrganizationID uuid.UUID
|
||||
ChatID uuid.UUID
|
||||
CreateFn CreateWorkspaceFn
|
||||
AgentConnFn AgentConnFunc
|
||||
@@ -142,6 +143,24 @@ func CreateWorkspace(options CreateWorkspaceOptions) fantasy.AgentTool {
|
||||
ctx = ownerCtx
|
||||
}
|
||||
|
||||
// Verify the template belongs to the same org as the
|
||||
// chat. Without this check the tool could silently
|
||||
// bind a cross-org workspace to the chat.
|
||||
if options.DB != nil && options.OrganizationID != uuid.Nil {
|
||||
tmpl, tmplErr := options.DB.GetTemplateByID(ctx, templateID)
|
||||
if tmplErr != nil {
|
||||
return fantasy.NewTextErrorResponse(
|
||||
xerrors.Errorf("look up template: %w", tmplErr).Error(),
|
||||
), nil
|
||||
}
|
||||
if tmpl.OrganizationID != options.OrganizationID {
|
||||
return fantasy.NewTextErrorResponse(
|
||||
"template belongs to a different organization than this chat; " +
|
||||
"use list_templates to find templates in the correct organization",
|
||||
), nil
|
||||
}
|
||||
}
|
||||
|
||||
var ttlMs *int64
|
||||
if options.DB != nil {
|
||||
raw, err := options.DB.GetChatWorkspaceTTL(ctx)
|
||||
@@ -168,7 +187,12 @@ func CreateWorkspace(options CreateWorkspaceOptions) fantasy.AgentTool {
|
||||
TTLMillis: ttlMs,
|
||||
}
|
||||
|
||||
// Resolve workspace name.
|
||||
// Resolve workspace name. This does a second
|
||||
// GetTemplateByID when no name is provided; the first
|
||||
// is the org-validation check above. Consolidating
|
||||
// them would couple the security gate to the
|
||||
// name-fallback path, and the cost is negligible next
|
||||
// to the workspace build that follows.
|
||||
name := strings.TrimSpace(args.Name)
|
||||
if name == "" {
|
||||
seed := "workspace"
|
||||
|
||||
@@ -507,6 +507,69 @@ func TestCreateWorkspace_GlobalTTL(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestCreateWorkspace_RejectsCrossOrgTemplate(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ctrl := gomock.NewController(t)
|
||||
db := dbmock.NewMockStore(ctrl)
|
||||
|
||||
ownerID := uuid.New()
|
||||
chatOrgID := uuid.New()
|
||||
templateOrgID := uuid.New() // Different org.
|
||||
templateID := uuid.New()
|
||||
|
||||
chatID := uuid.New()
|
||||
|
||||
// Chat exists but has no workspace binding.
|
||||
db.EXPECT().
|
||||
GetChatByID(gomock.Any(), chatID).
|
||||
Return(database.Chat{
|
||||
ID: chatID,
|
||||
WorkspaceID: uuid.NullUUID{},
|
||||
}, nil)
|
||||
|
||||
db.EXPECT().
|
||||
GetAuthorizationUserRoles(gomock.Any(), ownerID).
|
||||
Return(database.GetAuthorizationUserRolesRow{
|
||||
ID: ownerID,
|
||||
Roles: []string{},
|
||||
Groups: []string{},
|
||||
Status: database.UserStatusActive,
|
||||
}, nil)
|
||||
|
||||
db.EXPECT().
|
||||
GetTemplateByID(gomock.Any(), templateID).
|
||||
Return(database.Template{
|
||||
ID: templateID,
|
||||
OrganizationID: templateOrgID,
|
||||
Name: "wrong-org-template",
|
||||
}, nil)
|
||||
|
||||
createCalled := false
|
||||
tool := CreateWorkspace(CreateWorkspaceOptions{
|
||||
DB: db,
|
||||
OwnerID: ownerID,
|
||||
OrganizationID: chatOrgID,
|
||||
ChatID: chatID,
|
||||
CreateFn: func(context.Context, uuid.UUID, codersdk.CreateWorkspaceRequest) (codersdk.Workspace, error) {
|
||||
createCalled = true
|
||||
return codersdk.Workspace{}, nil
|
||||
},
|
||||
WorkspaceMu: &sync.Mutex{},
|
||||
Logger: slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}),
|
||||
})
|
||||
|
||||
input := fmt.Sprintf(`{"template_id":%q}`, templateID.String())
|
||||
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
|
||||
ID: "call-1",
|
||||
Name: "create_workspace",
|
||||
Input: input,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.False(t, createCalled, "CreateFn must not be called for cross-org template")
|
||||
require.Contains(t, resp.Content, "organization")
|
||||
}
|
||||
|
||||
func TestCheckExistingWorkspace_ConnectedAgent(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctrl := gomock.NewController(t)
|
||||
|
||||
@@ -24,6 +24,7 @@ const listTemplatesPageSize = 10
|
||||
type ListTemplatesOptions struct {
|
||||
DB database.Store
|
||||
OwnerID uuid.UUID
|
||||
OrganizationID uuid.UUID
|
||||
AllowedTemplateIDs func() map[uuid.UUID]bool
|
||||
}
|
||||
|
||||
@@ -55,7 +56,8 @@ func ListTemplates(options ListTemplatesOptions) fantasy.AgentTool {
|
||||
}
|
||||
|
||||
filterParams := database.GetTemplatesWithFilterParams{
|
||||
Deleted: false,
|
||||
Deleted: false,
|
||||
OrganizationID: options.OrganizationID,
|
||||
Deprecated: sql.NullBool{
|
||||
Bool: false,
|
||||
Valid: true,
|
||||
@@ -121,8 +123,9 @@ func ListTemplates(options ListTemplatesOptions) fantasy.AgentTool {
|
||||
items := make([]map[string]any, 0, len(pageTemplates))
|
||||
for _, t := range pageTemplates {
|
||||
item := map[string]any{
|
||||
"id": t.ID.String(),
|
||||
"name": t.Name,
|
||||
"id": t.ID.String(),
|
||||
"name": t.Name,
|
||||
"organization_id": t.OrganizationID.String(),
|
||||
}
|
||||
if display := strings.TrimSpace(t.DisplayName); display != "" {
|
||||
item["display_name"] = display
|
||||
|
||||
@@ -35,8 +35,14 @@ func TestStartWorkspace(t *testing.T) {
|
||||
|
||||
user := dbgen.User(t, db, database.User{})
|
||||
modelCfg := seedModelConfig(ctx, t, db, user.ID)
|
||||
org := dbgen.Organization(t, db, database.Organization{})
|
||||
_ = dbgen.OrganizationMember(t, db, database.OrganizationMember{
|
||||
UserID: user.ID,
|
||||
OrganizationID: org.ID,
|
||||
})
|
||||
|
||||
chat, err := db.InsertChat(ctx, database.InsertChatParams{
|
||||
OrganizationID: org.ID,
|
||||
Status: database.ChatStatusWaiting,
|
||||
OwnerID: user.ID,
|
||||
LastModelConfigID: modelCfg.ID,
|
||||
@@ -80,6 +86,7 @@ func TestStartWorkspace(t *testing.T) {
|
||||
ws := wsResp.Workspace
|
||||
|
||||
chat, err := db.InsertChat(ctx, database.InsertChatParams{
|
||||
OrganizationID: org.ID,
|
||||
Status: database.ChatStatusWaiting,
|
||||
OwnerID: user.ID,
|
||||
WorkspaceID: uuid.NullUUID{UUID: ws.ID, Valid: true},
|
||||
@@ -161,6 +168,7 @@ func TestStartWorkspace(t *testing.T) {
|
||||
require.NotEqual(t, uuid.Nil, preferredAgentID)
|
||||
|
||||
chat, err := db.InsertChat(ctx, database.InsertChatParams{
|
||||
OrganizationID: org.ID,
|
||||
Status: database.ChatStatusWaiting,
|
||||
OwnerID: user.ID,
|
||||
WorkspaceID: uuid.NullUUID{UUID: ws.ID, Valid: true},
|
||||
@@ -221,6 +229,7 @@ func TestStartWorkspace(t *testing.T) {
|
||||
ws := wsResp.Workspace
|
||||
|
||||
chat, err := db.InsertChat(ctx, database.InsertChatParams{
|
||||
OrganizationID: org.ID,
|
||||
Status: database.ChatStatusWaiting,
|
||||
OwnerID: user.ID,
|
||||
WorkspaceID: uuid.NullUUID{UUID: ws.ID, Valid: true},
|
||||
@@ -284,6 +293,7 @@ func TestStartWorkspace(t *testing.T) {
|
||||
ws := wsResp.Workspace
|
||||
|
||||
chat, err := db.InsertChat(ctx, database.InsertChatParams{
|
||||
OrganizationID: org.ID,
|
||||
Status: database.ChatStatusWaiting,
|
||||
OwnerID: user.ID,
|
||||
WorkspaceID: uuid.NullUUID{UUID: ws.ID, Valid: true},
|
||||
@@ -341,6 +351,7 @@ func TestStartWorkspace(t *testing.T) {
|
||||
ws := wsResp.Workspace
|
||||
|
||||
chat, err := db.InsertChat(ctx, database.InsertChatParams{
|
||||
OrganizationID: org.ID,
|
||||
Status: database.ChatStatusWaiting,
|
||||
OwnerID: user.ID,
|
||||
WorkspaceID: uuid.NullUUID{UUID: ws.ID, Valid: true},
|
||||
@@ -412,6 +423,7 @@ func TestStartWorkspace(t *testing.T) {
|
||||
ws := wsResp.Workspace
|
||||
|
||||
chat, err := db.InsertChat(ctx, database.InsertChatParams{
|
||||
OrganizationID: org.ID,
|
||||
Status: database.ChatStatusWaiting,
|
||||
OwnerID: user.ID,
|
||||
WorkspaceID: uuid.NullUUID{UUID: ws.ID, Valid: true},
|
||||
@@ -507,6 +519,7 @@ func TestStartWorkspace(t *testing.T) {
|
||||
ws := wsResp.Workspace
|
||||
|
||||
chat, err := db.InsertChat(ctx, database.InsertChatParams{
|
||||
OrganizationID: org.ID,
|
||||
Status: database.ChatStatusWaiting,
|
||||
OwnerID: user.ID,
|
||||
WorkspaceID: uuid.NullUUID{UUID: ws.ID, Valid: true},
|
||||
@@ -588,6 +601,7 @@ func TestStartWorkspace(t *testing.T) {
|
||||
ws := wsResp.Workspace
|
||||
|
||||
chat, err := db.InsertChat(ctx, database.InsertChatParams{
|
||||
OrganizationID: org.ID,
|
||||
Status: database.ChatStatusWaiting,
|
||||
OwnerID: user.ID,
|
||||
WorkspaceID: uuid.NullUUID{UUID: ws.ID, Valid: true},
|
||||
@@ -685,6 +699,7 @@ func TestStartWorkspace(t *testing.T) {
|
||||
ws := wsResp.Workspace
|
||||
|
||||
chat, err := db.InsertChat(ctx, database.InsertChatParams{
|
||||
OrganizationID: org.ID,
|
||||
Status: database.ChatStatusWaiting,
|
||||
OwnerID: user.ID,
|
||||
WorkspaceID: uuid.NullUUID{UUID: ws.ID, Valid: true},
|
||||
|
||||
Reference in New Issue
Block a user