From b5a625549e3fd45dfce6dad6f92a2f8082003d3b Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 23 Apr 2026 17:59:42 +0100 Subject: [PATCH] feat: migrate agents-access to org-scoped system role for proper chat RBAC (#24438) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The agents-access role previously granted chat permissions at user scope, but chats are org-scoped objects. Rego skips user-level perms when org_owner is set, making the grants invisible. Handler-level band-aids used synthetic non-org-scoped objects as a workaround. - Migrates agents-access from users.rbac_roles (site-level) to organization_members.roles (org-scoped) via DB migration - Redefines agents-access as a predefined org-scoped builtin role alongside organization-admin, organization-auditor, etc., with Member permissions granting chat create/read/update - Excludes ResourceChat from OrgMemberPermissions so org membership alone no longer grants chat access - Fixes handler Authorize checks to use org-scoped objects with semantically correct actions (ActionUpdate for message/tool operations) - Grants org admins the ability to assign agents-access Closes #24250 Fixes CODAGT-174 Note: this does not update the "Usage" endpoints. Tracked by CODAGT-161. > 🤖 --- coderd/database/dbauthz/dbauthz.go | 29 ++-- coderd/database/dbauthz/dbauthz_test.go | 8 +- coderd/database/dbauthz/setup_test.go | 1 + .../000475_agents_access_org_role.down.sql | 18 ++ .../000475_agents_access_org_role.up.sql | 16 ++ coderd/database/migrations/migrate_test.go | 162 ++++++++++++++++++ coderd/database/querier_test.go | 18 +- coderd/exp_chats.go | 52 +++--- coderd/exp_chats_test.go | 109 +++++++----- coderd/rbac/roles.go | 73 ++++---- coderd/rbac/roles_test.go | 43 +++-- coderd/roles.go | 7 - enterprise/coderd/exp_chats_test.go | 22 ++- enterprise/coderd/roles_test.go | 6 +- site/permissions.json | 2 +- site/site_test.go | 15 +- .../UserTable/EditRolesButton.stories.tsx | 22 +++ .../UserTable/EditRolesButton.tsx | 2 +- site/src/testHelpers/entities.ts | 10 ++ 19 files changed, 441 insertions(+), 174 deletions(-) create mode 100644 coderd/database/migrations/000475_agents_access_org_role.down.sql create mode 100644 coderd/database/migrations/000475_agents_access_org_role.up.sql diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 8ae54e845a..1fb81b96a5 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -2566,14 +2566,20 @@ func (q *querier) GetChatByIDForUpdate(ctx context.Context, id uuid.UUID) (datab } func (q *querier) GetChatCostPerChat(ctx context.Context, arg database.GetChatCostPerChatParams) ([]database.GetChatCostPerChatRow, error) { - if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceChat.WithOwner(arg.OwnerID.String())); err != nil { + // The owner's chats, may cross orgs. AnyOrganization() authorizes + // the caller if they hold read permission on chats owned by + // arg.OwnerID in any org they belong to. + // TODO(CODAGT-161): the underlying SQL queries filter only by owner_id, not + // organization_id. + if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceChat.WithOwner(arg.OwnerID.String()).AnyOrganization()); err != nil { return nil, err } return q.db.GetChatCostPerChat(ctx, arg) } func (q *querier) GetChatCostPerModel(ctx context.Context, arg database.GetChatCostPerModelParams) ([]database.GetChatCostPerModelRow, error) { - if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceChat.WithOwner(arg.OwnerID.String())); err != nil { + // See GetChatCostPerChat for the authorization rationale. + if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceChat.WithOwner(arg.OwnerID.String()).AnyOrganization()); err != nil { return nil, err } return q.db.GetChatCostPerModel(ctx, arg) @@ -2587,7 +2593,8 @@ func (q *querier) GetChatCostPerUser(ctx context.Context, arg database.GetChatCo } func (q *querier) GetChatCostSummary(ctx context.Context, arg database.GetChatCostSummaryParams) (database.GetChatCostSummaryRow, error) { - if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceChat.WithOwner(arg.OwnerID.String())); err != nil { + // See GetChatCostPerChat for the authorization rationale. + if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceChat.WithOwner(arg.OwnerID.String()).AnyOrganization()); err != nil { return database.GetChatCostSummaryRow{}, err } return q.db.GetChatCostSummary(ctx, arg) @@ -3025,17 +3032,15 @@ func (q *querier) GetDERPMeshKey(ctx context.Context) (string, error) { } func (q *querier) GetDefaultChatModelConfig(ctx context.Context) (database.ChatModelConfig, error) { - // Any user who can read chat resources can read the default - // model config, since model resolution is required to create - // a chat. This avoids gating on ResourceDeploymentConfig - // which regular members lack. - act, ok := ActorFromContext(ctx) - if !ok { + // Reading the default model config is needed for chat creation. + // TODO(CODAGT-161): scope this check when org context is available. + // This function has no org context to scope the check, and + // ResourceDeploymentConfig is too restrictive (admin-only). + // The handler layer gates chat creation via ActionCreate on + // the org-scoped ResourceChat. + if _, ok := ActorFromContext(ctx); !ok { return database.ChatModelConfig{}, ErrNoActor } - if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceChat.WithOwner(act.ID)); err != nil { - return database.ChatModelConfig{}, err - } return q.db.GetDefaultChatModelConfig(ctx) } diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 7a7ee8f219..1d3b9530c9 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -618,7 +618,7 @@ func (s *MethodTestSuite) TestChats() { TotalOutputTokens: 89, }} dbm.EXPECT().GetChatCostPerChat(gomock.Any(), arg).Return(rows, nil).AnyTimes() - check.Args(arg).Asserts(rbac.ResourceChat.WithOwner(arg.OwnerID.String()), policy.ActionRead).Returns(rows) + check.Args(arg).Asserts(rbac.ResourceChat.WithOwner(arg.OwnerID.String()).AnyOrganization(), policy.ActionRead).Returns(rows) })) s.Run("GetChatCostPerModel", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) { arg := database.GetChatCostPerModelParams{ @@ -637,7 +637,7 @@ func (s *MethodTestSuite) TestChats() { TotalOutputTokens: 233, }} dbm.EXPECT().GetChatCostPerModel(gomock.Any(), arg).Return(rows, nil).AnyTimes() - check.Args(arg).Asserts(rbac.ResourceChat.WithOwner(arg.OwnerID.String()), policy.ActionRead).Returns(rows) + check.Args(arg).Asserts(rbac.ResourceChat.WithOwner(arg.OwnerID.String()).AnyOrganization(), policy.ActionRead).Returns(rows) })) s.Run("GetChatCostPerUser", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) { arg := database.GetChatCostPerUserParams{ @@ -676,7 +676,7 @@ func (s *MethodTestSuite) TestChats() { TotalOutputTokens: 800, } dbm.EXPECT().GetChatCostSummary(gomock.Any(), arg).Return(row, nil).AnyTimes() - check.Args(arg).Asserts(rbac.ResourceChat.WithOwner(arg.OwnerID.String()), policy.ActionRead).Returns(row) + check.Args(arg).Asserts(rbac.ResourceChat.WithOwner(arg.OwnerID.String()).AnyOrganization(), policy.ActionRead).Returns(row) })) s.Run("CountEnabledModelsWithoutPricing", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) { dbm.EXPECT().CountEnabledModelsWithoutPricing(gomock.Any()).Return(int64(3), nil).AnyTimes() @@ -795,7 +795,7 @@ func (s *MethodTestSuite) TestChats() { s.Run("GetDefaultChatModelConfig", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { config := testutil.Fake(s.T(), faker, database.ChatModelConfig{}) dbm.EXPECT().GetDefaultChatModelConfig(gomock.Any()).Return(config, nil).AnyTimes() - check.Asserts(rbac.ResourceChat.WithOwner(testActorID.String()), policy.ActionRead).Returns(config) + check.Asserts().Returns(config) })) s.Run("GetChatModelConfigs", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { configA := testutil.Fake(s.T(), faker, database.ChatModelConfig{}) diff --git a/coderd/database/dbauthz/setup_test.go b/coderd/database/dbauthz/setup_test.go index 3a100d7e1b..bab2cac91c 100644 --- a/coderd/database/dbauthz/setup_test.go +++ b/coderd/database/dbauthz/setup_test.go @@ -242,6 +242,7 @@ func (s *MethodTestSuite) SubtestWithDB(db database.Store, testCaseF func(db dat slice.Contains([]string{ "GetAuthorizedWorkspaces", "GetAuthorizedTemplates", + "GetDefaultChatModelConfig", }, methodName) { // Some methods do not make RBAC assertions because they use // SQL. We still want to test that they return an error if the diff --git a/coderd/database/migrations/000475_agents_access_org_role.down.sql b/coderd/database/migrations/000475_agents_access_org_role.down.sql new file mode 100644 index 0000000000..80582be2c7 --- /dev/null +++ b/coderd/database/migrations/000475_agents_access_org_role.down.sql @@ -0,0 +1,18 @@ +-- WARNING: this rollback is lossy. If an admin later revoked +-- agents-access from a specific org, rolling back will re-grant the +-- site-wide role (which covers ALL orgs) to any user who still holds +-- agents-access in at least one org. + +-- Step 1: Move agents-access back to site-level for any user who has it in any org. +UPDATE users +SET rbac_roles = array_append(rbac_roles, 'agents-access') +WHERE id IN ( + SELECT DISTINCT user_id FROM organization_members + WHERE 'agents-access' = ANY(roles) +) +AND NOT ('agents-access' = ANY(rbac_roles)); + +-- Step 2: Remove from org memberships. +UPDATE organization_members +SET roles = array_remove(roles, 'agents-access') +WHERE 'agents-access' = ANY(roles); diff --git a/coderd/database/migrations/000475_agents_access_org_role.up.sql b/coderd/database/migrations/000475_agents_access_org_role.up.sql new file mode 100644 index 0000000000..96212dd615 --- /dev/null +++ b/coderd/database/migrations/000475_agents_access_org_role.up.sql @@ -0,0 +1,16 @@ +-- Transition 'agents-access' from a site-wide role to a per-org role. + +-- For every user who has 'agents-access' in users.rbac_roles, +-- grant the org-scoped role in each org they belong to. +UPDATE organization_members +SET roles = array_append(roles, 'agents-access') +WHERE user_id IN ( + SELECT id FROM users + WHERE 'agents-access' = ANY(rbac_roles) +) +AND NOT ('agents-access' = ANY(roles)); + +-- Remove 'agents-access' from site-level roles. +UPDATE users +SET rbac_roles = array_remove(rbac_roles, 'agents-access') +WHERE 'agents-access' = ANY(rbac_roles); diff --git a/coderd/database/migrations/migrate_test.go b/coderd/database/migrations/migrate_test.go index 723fd66ae2..ae16886661 100644 --- a/coderd/database/migrations/migrate_test.go +++ b/coderd/database/migrations/migrate_test.go @@ -1023,3 +1023,165 @@ func TestMigration000457ChatAccessRole(t *testing.T) { require.Contains(t, roles, "template-admin", "existing roles should be preserved") } + +func TestMigration000475AgentsAccessOrgRole(t *testing.T) { + t.Parallel() + + const migrationVersion = 475 + + sqlDB := testSQLDB(t) + + // Migrate up to the migration before 000475. + next, err := migrations.Stepper(sqlDB) + require.NoError(t, err) + for { + version, more, err := next() + require.NoError(t, err) + if !more { + t.Fatalf("migration %d not found", migrationVersion) + } + if version == migrationVersion-1 { + break + } + } + + ctx := testutil.Context(t, testutil.WaitSuperLong) + + // Seed: a user with site-level agents-access who is a member of + // two orgs, plus a second user who is a member of one org and + // does not have the role. + userWithRole := uuid.New() + userWithoutRole := uuid.New() + org1ID := uuid.New() + org2ID := uuid.New() + + now := time.Now().UTC().Truncate(time.Microsecond) + + tx, err := sqlDB.BeginTx(ctx, nil) + require.NoError(t, err) + defer tx.Rollback() + + fixtures := []struct { + query string + args []any + }{ + { + `INSERT INTO users (id, username, email, hashed_password, created_at, updated_at, status, rbac_roles, login_type) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)`, + []any{userWithRole, "user-with-role", "withrole@test.com", []byte{}, now, now, "active", pq.StringArray{"agents-access"}, "password"}, + }, + { + `INSERT INTO users (id, username, email, hashed_password, created_at, updated_at, status, rbac_roles, login_type) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)`, + []any{userWithoutRole, "user-without-role", "withoutrole@test.com", []byte{}, now, now, "active", pq.StringArray{}, "password"}, + }, + { + `INSERT INTO organizations (id, name, display_name, description, icon, created_at, updated_at, is_default) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8)`, + []any{org1ID, "org-1", "Org 1", "", "", now, now, false}, + }, + { + `INSERT INTO organizations (id, name, display_name, description, icon, created_at, updated_at, is_default) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8)`, + []any{org2ID, "org-2", "Org 2", "", "", now, now, false}, + }, + { + `INSERT INTO organization_members (organization_id, user_id, created_at, updated_at, roles) + VALUES ($1, $2, $3, $4, $5)`, + []any{org1ID, userWithRole, now, now, pq.StringArray{}}, + }, + { + `INSERT INTO organization_members (organization_id, user_id, created_at, updated_at, roles) + VALUES ($1, $2, $3, $4, $5)`, + []any{org2ID, userWithRole, now, now, pq.StringArray{}}, + }, + { + `INSERT INTO organization_members (organization_id, user_id, created_at, updated_at, roles) + VALUES ($1, $2, $3, $4, $5)`, + []any{org1ID, userWithoutRole, now, now, pq.StringArray{}}, + }, + } + + for i, f := range fixtures { + _, err := tx.ExecContext(ctx, f.query, f.args...) + require.NoError(t, err, "fixture %d", i) + } + require.NoError(t, tx.Commit()) + + // Run migration 000475. + version, _, err := next() + require.NoError(t, err) + require.EqualValues(t, migrationVersion, version) + + // Verify: userWithRole no longer has agents-access at site level. + var siteRoles pq.StringArray + err = sqlDB.QueryRowContext(ctx, + "SELECT rbac_roles FROM users WHERE id = $1", userWithRole, + ).Scan(&siteRoles) + require.NoError(t, err) + require.NotContains(t, siteRoles, "agents-access", + "agents-access should be removed from users.rbac_roles") + + // Verify: userWithRole has agents-access in both orgs. + for _, orgID := range []uuid.UUID{org1ID, org2ID} { + var orgRoles pq.StringArray + err = sqlDB.QueryRowContext(ctx, + "SELECT roles FROM organization_members WHERE user_id = $1 AND organization_id = $2", + userWithRole, orgID, + ).Scan(&orgRoles) + require.NoError(t, err) + require.Contains(t, orgRoles, "agents-access", + "agents-access should be granted in org %s", orgID) + } + + // Verify: userWithoutRole did not gain agents-access. + var orgRoles pq.StringArray + err = sqlDB.QueryRowContext(ctx, + "SELECT roles FROM organization_members WHERE user_id = $1 AND organization_id = $2", + userWithoutRole, org1ID, + ).Scan(&orgRoles) + require.NoError(t, err) + require.NotContains(t, orgRoles, "agents-access", + "agents-access should not be granted to a user who didn't have it") + + // Verify: no DB row exists for agents-access as a custom_role. + // The role is now a builtin, resolved in Go via RoleByName. + var customRoleCount int + err = sqlDB.QueryRowContext(ctx, + "SELECT COUNT(*) FROM custom_roles WHERE name = 'agents-access'", + ).Scan(&customRoleCount) + require.NoError(t, err) + require.Equal(t, 0, customRoleCount, + "no custom_roles row should exist for agents-access") + + // Verify: creating a new organization does NOT insert an + // agents-access custom_role via the trigger. It should only + // insert organization-member and organization-service-account. + newOrgID := uuid.New() + _, err = sqlDB.ExecContext(ctx, + `INSERT INTO organizations (id, name, display_name, description, icon, created_at, updated_at, is_default) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8)`, + newOrgID, "new-org", "New Org", "", "", now, now, false, + ) + require.NoError(t, err) + + rows, err := sqlDB.QueryContext(ctx, + "SELECT name FROM custom_roles WHERE organization_id = $1 AND is_system = true ORDER BY name", + newOrgID, + ) + require.NoError(t, err) + defer rows.Close() + + var gotRoleNames []string + for rows.Next() { + var name string + require.NoError(t, rows.Scan(&name)) + gotRoleNames = append(gotRoleNames, name) + } + require.NoError(t, rows.Err()) + require.ElementsMatch(t, + []string{"organization-member", "organization-service-account"}, + gotRoleNames, + "trigger should only create org-member and org-service-account system roles", + ) +} diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index fd906b7bae..1817fa241a 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -1251,17 +1251,13 @@ func TestGetAuthorizedChats(t *testing.T) { owner := dbgen.User(t, db, database.User{ RBACRoles: []string{rbac.RoleOwner().String()}, }) - member := dbgen.User(t, db, database.User{ - RBACRoles: pq.StringArray{rbac.RoleAgentsAccess().String()}, - }) - secondMember := dbgen.User(t, db, database.User{ - RBACRoles: pq.StringArray{rbac.RoleAgentsAccess().String()}, - }) + member := dbgen.User(t, db, database.User{}) + secondMember := dbgen.User(t, db, database.User{}) org := dbgen.Organization(t, db, database.Organization{}) dbgen.OrganizationMember(t, db, database.OrganizationMember{UserID: owner.ID, OrganizationID: org.ID}) - dbgen.OrganizationMember(t, db, database.OrganizationMember{UserID: member.ID, OrganizationID: org.ID}) - dbgen.OrganizationMember(t, db, database.OrganizationMember{UserID: secondMember.ID, OrganizationID: org.ID}) + dbgen.OrganizationMember(t, db, database.OrganizationMember{UserID: member.ID, OrganizationID: org.ID, Roles: []string{rbac.RoleAgentsAccess()}}) + dbgen.OrganizationMember(t, db, database.OrganizationMember{UserID: secondMember.ID, OrganizationID: org.ID, Roles: []string{rbac.RoleAgentsAccess()}}) // Create FK dependencies: a chat provider and model config. ctx := testutil.Context(t, testutil.WaitMedium) @@ -1438,10 +1434,8 @@ func TestGetAuthorizedChats(t *testing.T) { // Use a dedicated user for pagination to avoid interference // with the other parallel subtests. - paginationUser := dbgen.User(t, db, database.User{ - RBACRoles: pq.StringArray{rbac.RoleAgentsAccess().String()}, - }) - dbgen.OrganizationMember(t, db, database.OrganizationMember{UserID: paginationUser.ID, OrganizationID: org.ID}) + paginationUser := dbgen.User(t, db, database.User{}) + dbgen.OrganizationMember(t, db, database.OrganizationMember{UserID: paginationUser.ID, OrganizationID: org.ID, Roles: []string{rbac.RoleAgentsAccess()}}) for i := range 7 { _, err := db.InsertChat(ctx, database.InsertChatParams{ OrganizationID: org.ID, diff --git a/coderd/exp_chats.go b/coderd/exp_chats.go index d6c1153a20..c9cca1b4c8 100644 --- a/coderd/exp_chats.go +++ b/coderd/exp_chats.go @@ -524,11 +524,6 @@ func (api *API) postChats(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() apiKey := httpmw.APIKey(r) - if !api.Authorize(r, policy.ActionCreate, rbac.ResourceChat.WithOwner(apiKey.UserID.String())) { - httpapi.Forbidden(rw) - return - } - // Cap the raw request body to prevent excessive memory use // from large dynamic tool schemas. r.Body = http.MaxBytesReader(rw, r.Body, int64(2*maxSystemPromptLenBytes)) @@ -568,6 +563,14 @@ func (api *API) postChats(rw http.ResponseWriter, r *http.Request) { }) return } + // NOTE: This authorize check is intentionally placed after request + // parsing because we need req.OrganizationID to scope the RBAC check + // to the correct org. The request body is bounded by MaxBytesReader + // above, limiting the cost of parsing before rejection. + if !api.Authorize(r, policy.ActionCreate, rbac.ResourceChat.WithOwner(apiKey.UserID.String()).InOrg(req.OrganizationID)) { + httpapi.Forbidden(rw) + return + } // Validate per-chat system prompt length. const maxSystemPromptLen = 10000 @@ -2344,14 +2347,9 @@ func (api *API) postChatMessages(rw http.ResponseWriter, r *http.Request) { chat := httpmw.ChatParam(r) chatID := chat.ID - // Gate message sending behind the same agents-access check - // used by postChats. Sending a message triggers AI/LLM - // inference, so it should require the same authorization as - // chat creation. This is a handler-level band-aid; the - // structural fix is to make agents-access org-aware so - // dbauthz enforces this at the RBAC layer. - // See: https://github.com/coder/coder/issues/24250 - if !api.Authorize(r, policy.ActionCreate, rbac.ResourceChat.WithOwner(apiKey.UserID.String())) { + // Sending a message triggers LLM inference, requiring update + // permission on the org-scoped chat resource. + if !api.Authorize(r, policy.ActionUpdate, chat.RBACObject()) { httpapi.Forbidden(rw) return } @@ -2640,11 +2638,9 @@ func (api *API) promoteChatQueuedMessage(rw http.ResponseWriter, r *http.Request chat := httpmw.ChatParam(r) chatID := chat.ID - // Gate queued-message promotion behind agents-access. - // Promoting a queued message triggers AI/LLM inference, - // same as sending a new message. - // See: https://github.com/coder/coder/issues/24250 - if !api.Authorize(r, policy.ActionCreate, rbac.ResourceChat.WithOwner(apiKey.UserID.String())) { + // Promoting a queued message triggers LLM inference, + // requiring update permission on the org-scoped chat resource. + if !api.Authorize(r, policy.ActionUpdate, chat.RBACObject()) { httpapi.Forbidden(rw) return } @@ -4567,11 +4563,6 @@ func (api *API) postChatFile(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() apiKey := httpmw.APIKey(r) - if !api.Authorize(r, policy.ActionCreate, rbac.ResourceChat.WithOwner(apiKey.UserID.String())) { - httpapi.Forbidden(rw) - return - } - orgIDStr := r.URL.Query().Get("organization") if orgIDStr == "" { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ @@ -4586,6 +4577,13 @@ func (api *API) postChatFile(rw http.ResponseWriter, r *http.Request) { }) return } + // NOTE: This authorize check is intentionally placed after query + // parameter parsing because we need orgID to scope the RBAC check + // to the correct org. + if !api.Authorize(r, policy.ActionCreate, rbac.ResourceChat.WithOwner(apiKey.UserID.String()).InOrg(orgID)) { + httpapi.Forbidden(rw) + return + } contentType := r.Header.Get("Content-Type") if contentType == "" { @@ -6754,11 +6752,9 @@ func (api *API) postChatToolResults(rw http.ResponseWriter, r *http.Request) { chat := httpmw.ChatParam(r) apiKey := httpmw.APIKey(r) - // Gate tool-result submission behind agents-access. - // Submitting tool results resumes AI/LLM inference on - // a chat in requires_action state. - // See: https://github.com/coder/coder/issues/24250 - if !api.Authorize(r, policy.ActionCreate, rbac.ResourceChat.WithOwner(apiKey.UserID.String())) { + // Submitting tool results resumes LLM inference, + // requiring update permission on the org-scoped chat resource. + if !api.Authorize(r, policy.ActionUpdate, chat.RBACObject()) { httpapi.Forbidden(rw) return } diff --git a/coderd/exp_chats_test.go b/coderd/exp_chats_test.go index 505838c270..ea35d4c4c0 100644 --- a/coderd/exp_chats_test.go +++ b/coderd/exp_chats_test.go @@ -269,7 +269,7 @@ func TestPostChats(t *testing.T) { // Use a member with agents-access instead of the owner to // verify least-privilege access. - memberClientRaw, member := coderdtest.CreateAnotherUser(t, client.Client, firstUser.OrganizationID, rbac.RoleAgentsAccess()) + memberClientRaw, member := coderdtest.CreateAnotherUser(t, client.Client, firstUser.OrganizationID, rbac.ScopedRoleAgentsAccess(firstUser.OrganizationID)) memberClient := codersdk.NewExperimentalClient(memberClientRaw) chat, err := memberClient.CreateChat(ctx, codersdk.CreateChatRequest{ @@ -484,7 +484,7 @@ func TestPostChats(t *testing.T) { ctx := testutil.Context(t, testutil.WaitLong) adminClient, db := newChatClientWithDatabase(t) firstUser := coderdtest.CreateFirstUser(t, adminClient.Client) - memberClientRaw, _ := coderdtest.CreateAnotherUser(t, adminClient.Client, firstUser.OrganizationID, rbac.RoleAgentsAccess()) + memberClientRaw, _ := coderdtest.CreateAnotherUser(t, adminClient.Client, firstUser.OrganizationID, rbac.ScopedRoleAgentsAccess(firstUser.OrganizationID)) memberClient := codersdk.NewExperimentalClient(memberClientRaw) workspaceBuild := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ @@ -521,7 +521,7 @@ func TestPostChats(t *testing.T) { adminClient.Client, firstUser.OrganizationID, rbac.ScopedRoleOrgAdmin(firstUser.OrganizationID), - rbac.RoleAgentsAccess(), + rbac.ScopedRoleAgentsAccess(firstUser.OrganizationID), ) orgAdminClient := codersdk.NewExperimentalClient(orgAdminClientRaw) @@ -720,7 +720,7 @@ func TestPostChats(t *testing.T) { firstUser := coderdtest.CreateFirstUser(t, client.Client) _ = createChatModelConfig(t, client) - memberClientRaw, _ := coderdtest.CreateAnotherUser(t, client.Client, firstUser.OrganizationID, rbac.RoleAgentsAccess()) + memberClientRaw, _ := coderdtest.CreateAnotherUser(t, client.Client, firstUser.OrganizationID, rbac.ScopedRoleAgentsAccess(firstUser.OrganizationID)) memberClient := codersdk.NewExperimentalClient(memberClientRaw) _, err := memberClient.CreateChat(ctx, codersdk.CreateChatRequest{ @@ -742,7 +742,7 @@ func TestPostChats(t *testing.T) { firstUser := coderdtest.CreateFirstUser(t, client.Client) _ = createChatModelConfig(t, client) - memberClientRaw, _ := coderdtest.CreateAnotherUser(t, client.Client, firstUser.OrganizationID, rbac.RoleAgentsAccess()) + memberClientRaw, _ := coderdtest.CreateAnotherUser(t, client.Client, firstUser.OrganizationID, rbac.ScopedRoleAgentsAccess(firstUser.OrganizationID)) memberClient := codersdk.NewExperimentalClient(memberClientRaw) // Create a second organization via the database since the @@ -802,7 +802,7 @@ func TestPostChats_ClientType(t *testing.T) { firstUser := coderdtest.CreateFirstUser(t, client.Client) _ = createChatModelConfig(t, client) - memberClientRaw, _ := coderdtest.CreateAnotherUser(t, client.Client, firstUser.OrganizationID, rbac.RoleAgentsAccess()) + memberClientRaw, _ := coderdtest.CreateAnotherUser(t, client.Client, firstUser.OrganizationID, rbac.ScopedRoleAgentsAccess(firstUser.OrganizationID)) memberClient := codersdk.NewExperimentalClient(memberClientRaw) newChat := func(t *testing.T, clientType codersdk.ChatClientType) codersdk.Chat { @@ -907,7 +907,7 @@ func TestListChats(t *testing.T) { }) require.NoError(t, err) - memberClientRaw, member := coderdtest.CreateAnotherUser(t, client.Client, firstUser.OrganizationID, rbac.RoleAgentsAccess()) + memberClientRaw, member := coderdtest.CreateAnotherUser(t, client.Client, firstUser.OrganizationID, rbac.ScopedRoleAgentsAccess(firstUser.OrganizationID)) memberClient := codersdk.NewExperimentalClient(memberClientRaw) memberDBChat, err := db.InsertChat(dbauthz.AsSystemRestricted(ctx), database.InsertChatParams{ OrganizationID: firstUser.OrganizationID, @@ -978,7 +978,7 @@ func TestListChats(t *testing.T) { require.Equal(t, memberChats[0].ID, memberChats[0].DiffStatus.ChatID) }) - t.Run("OrgMemberWithoutAgentsAccessCanAccessOwnChats", func(t *testing.T) { + t.Run("OrgMemberWithoutAgentsAccessCannotAccessOwnChats", func(t *testing.T) { t.Parallel() ctx := testutil.Context(t, testutil.WaitLong) client, db := newChatClientWithDatabase(t) @@ -986,16 +986,13 @@ func TestListChats(t *testing.T) { modelConfig := createChatModelConfig(t, client) // Create a member without agents-access and insert a chat - // owned by them via system context. With org-scoped chats, - // org members get full CRUD on their own chats through - // OrgMemberPermissions, without needing agents-access. - // The agents-access role only gates chat creation (postChats) - // and message sending (postChatMessages). Metadata operations - // like archive/pin/label and reading are not gated. - // See: https://github.com/coder/coder/issues/24250 + // owned by them via system context. Without agents-access, + // the member has no ResourceChat permissions at all, so + // listing returns 0 chats (SQL auth filter) and getting + // a specific chat returns 404 (dbauthz wraps as not found). memberClientRaw, member := coderdtest.CreateAnotherUser(t, client.Client, firstUser.OrganizationID) memberClient := codersdk.NewExperimentalClient(memberClientRaw) - _, err := db.InsertChat(dbauthz.AsSystemRestricted(ctx), database.InsertChatParams{ + chat, err := db.InsertChat(dbauthz.AsSystemRestricted(ctx), database.InsertChatParams{ OrganizationID: firstUser.OrganizationID, Status: database.ChatStatusWaiting, ClientType: database.ChatClientTypeUi, @@ -1005,15 +1002,18 @@ func TestListChats(t *testing.T) { }) require.NoError(t, err) + // Listing chats returns empty because the SQL auth + // filter excludes chats the member cannot read. chats, err := memberClient.ListChats(ctx, nil) require.NoError(t, err) - require.Len(t, chats, 1) + require.Len(t, chats, 0) - // Verify member without agents-access can update own chat. - err = memberClient.UpdateChat(ctx, chats[0].ID, codersdk.UpdateChatRequest{ + // Getting a specific chat returns 404 because dbauthz + // wraps authorization failures as not-found. + err = memberClient.UpdateChat(ctx, chat.ID, codersdk.UpdateChatRequest{ Title: ptr.Ref("new title"), }) - require.NoError(t, err) + requireSDKError(t, err, http.StatusNotFound) }) t.Run("Unauthenticated", func(t *testing.T) { @@ -4127,7 +4127,7 @@ func TestGetChat(t *testing.T) { }) require.NoError(t, err) - otherClientRaw, _ := coderdtest.CreateAnotherUser(t, client.Client, firstUser.OrganizationID, rbac.RoleAgentsAccess()) + otherClientRaw, _ := coderdtest.CreateAnotherUser(t, client.Client, firstUser.OrganizationID, rbac.ScopedRoleAgentsAccess(firstUser.OrganizationID)) otherClient := codersdk.NewExperimentalClient(otherClientRaw) _, err = otherClient.GetChat(ctx, createdChat.ID) requireSDKError(t, err, http.StatusNotFound) @@ -5680,10 +5680,10 @@ func TestPostChatMessages(t *testing.T) { modelConfig := createChatModelConfig(t, client) // Create a member without agents-access and insert a - // chat owned by them via system context. Even though - // the member can read the chat through org membership, - // sending messages should be gated by agents-access - // because it triggers AI/LLM inference. + // chat owned by them via system context. Without + // agents-access the member has no ResourceChat + // permissions, so the ChatParam middleware returns 404 + // before the handler can check agents-access. memberClientRaw, member := coderdtest.CreateAnotherUser(t, client.Client, firstUser.OrganizationID) memberClient := codersdk.NewExperimentalClient(memberClientRaw) chat, err := db.InsertChat(dbauthz.AsSystemRestricted(ctx), database.InsertChatParams{ @@ -5704,7 +5704,7 @@ func TestPostChatMessages(t *testing.T) { }, }, }) - requireSDKError(t, err, http.StatusForbidden) + requireSDKError(t, err, http.StatusNotFound) }) t.Run("EmptyText", func(t *testing.T) { @@ -7230,7 +7230,7 @@ func TestRegenerateChatTitle(t *testing.T) { }) require.NoError(t, err) - otherClientRaw, _ := coderdtest.CreateAnotherUser(t, client.Client, firstUser.OrganizationID, rbac.RoleAgentsAccess()) + otherClientRaw, _ := coderdtest.CreateAnotherUser(t, client.Client, firstUser.OrganizationID, rbac.ScopedRoleAgentsAccess(firstUser.OrganizationID)) otherClient := codersdk.NewExperimentalClient(otherClientRaw) _, err = otherClient.RegenerateChatTitle(ctx, createdChat.ID) requireSDKError(t, err, http.StatusNotFound) @@ -7656,7 +7656,7 @@ func TestGetChatDiffStatus(t *testing.T) { }) require.NoError(t, err) - otherClientRaw, _ := coderdtest.CreateAnotherUser(t, client.Client, firstUser.OrganizationID, rbac.RoleAgentsAccess()) + otherClientRaw, _ := coderdtest.CreateAnotherUser(t, client.Client, firstUser.OrganizationID, rbac.ScopedRoleAgentsAccess(firstUser.OrganizationID)) otherClient := codersdk.NewExperimentalClient(otherClientRaw) _, err = otherClient.GetChat(ctx, createdChat.ID) requireSDKError(t, err, http.StatusNotFound) @@ -7767,7 +7767,7 @@ func TestGetChatDiffContents(t *testing.T) { }) require.NoError(t, err) - otherClientRaw, _ := coderdtest.CreateAnotherUser(t, client.Client, firstUser.OrganizationID, rbac.RoleAgentsAccess()) + otherClientRaw, _ := coderdtest.CreateAnotherUser(t, client.Client, firstUser.OrganizationID, rbac.ScopedRoleAgentsAccess(firstUser.OrganizationID)) otherClient := codersdk.NewExperimentalClient(otherClientRaw) _, err = otherClient.GetChatDiffContents(ctx, createdChat.ID) requireSDKError(t, err, http.StatusNotFound) @@ -8059,9 +8059,10 @@ func TestPromoteChatQueuedMessage(t *testing.T) { firstUser := coderdtest.CreateFirstUser(t, client.Client) modelConfig := createChatModelConfig(t, client) - // Create a member without agents-access. Even though the - // member owns the chat, promoting a queued message should - // be gated by agents-access because it triggers inference. + // Create a member without agents-access. Without + // agents-access the member has no ResourceChat + // permissions, so the ChatParam middleware returns 404 + // before the handler can check agents-access. memberClientRaw, member := coderdtest.CreateAnotherUser(t, client.Client, firstUser.OrganizationID) memberClient := codersdk.NewExperimentalClient(memberClientRaw) chat, err := db.InsertChat(dbauthz.AsSystemRestricted(ctx), database.InsertChatParams{ @@ -8095,7 +8096,7 @@ func TestPromoteChatQueuedMessage(t *testing.T) { ) require.NoError(t, err) defer promoteRes.Body.Close() - require.Equal(t, http.StatusForbidden, promoteRes.StatusCode) + require.Equal(t, http.StatusNotFound, promoteRes.StatusCode) }) t.Run("ArchivedChat", func(t *testing.T) { @@ -8550,6 +8551,22 @@ widgets,3 _, err := unauthed.UploadChatFile(ctx, firstUser.OrganizationID, "image/png", "test.png", bytes.NewReader(data)) requireSDKError(t, err, http.StatusUnauthorized) }) + + t.Run("MemberWithoutAgentsAccess", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + client := newChatClient(t) + firstUser := coderdtest.CreateFirstUser(t, client.Client) + + // Member without agents-access should be denied. + memberClientRaw, _ := coderdtest.CreateAnotherUser(t, client.Client, firstUser.OrganizationID) + memberClient := codersdk.NewExperimentalClient(memberClientRaw) + + data := append([]byte{0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A}, make([]byte, 64)...) + _, err := memberClient.UploadChatFile(ctx, firstUser.OrganizationID, "image/png", "test.png", bytes.NewReader(data)) + requireSDKError(t, err, http.StatusForbidden) + }) } func TestGetChatFile(t *testing.T) { @@ -8696,7 +8713,7 @@ func TestGetChatFile(t *testing.T) { uploaded, err := client.UploadChatFile(ctx, firstUser.OrganizationID, "image/png", "test.png", bytes.NewReader(data)) require.NoError(t, err) - otherClientRaw, _ := coderdtest.CreateAnotherUser(t, client.Client, firstUser.OrganizationID, rbac.RoleAgentsAccess()) + otherClientRaw, _ := coderdtest.CreateAnotherUser(t, client.Client, firstUser.OrganizationID, rbac.ScopedRoleAgentsAccess(firstUser.OrganizationID)) otherClient := codersdk.NewExperimentalClient(otherClientRaw) _, _, err = otherClient.GetChatFile(ctx, uploaded.ID) requireSDKError(t, err, http.StatusNotFound) @@ -9304,10 +9321,15 @@ func TestWatchChatGitAuthz(t *testing.T) { // Demote adminClient via the second owner. template-admin grants // workspace:read (site) but not workspace:ssh or // workspace:application_connect; agents-access preserves - // chat:read|create|update|delete on chats the user owns, so the + // chat:create|read|update on chats the user owns, so the // demoted user still passes ExtractChatParam for their own chat. _, err = secondAdminClient.UpdateUserRoles(ctx, firstUser.UserID.String(), codersdk.UpdateRoles{ - Roles: []string{rbac.RoleTemplateAdmin().String(), rbac.RoleAgentsAccess().String()}, + Roles: []string{rbac.RoleTemplateAdmin().String()}, + }) + require.NoError(t, err) + + _, err = secondAdminClient.UpdateOrganizationMemberRoles(ctx, firstUser.OrganizationID, firstUser.UserID.String(), codersdk.UpdateRoles{ + Roles: []string{rbac.RoleAgentsAccess()}, }) require.NoError(t, err) @@ -10305,7 +10327,7 @@ func TestChatDebugRuns(t *testing.T) { firstUser := coderdtest.CreateFirstUser(t, client.Client) modelConfig := createChatModelConfig(t, client) - memberClientRaw, member := coderdtest.CreateAnotherUser(t, client.Client, firstUser.OrganizationID, rbac.RoleAgentsAccess()) + memberClientRaw, member := coderdtest.CreateAnotherUser(t, client.Client, firstUser.OrganizationID, rbac.ScopedRoleAgentsAccess(firstUser.OrganizationID)) memberClient := codersdk.NewExperimentalClient(memberClientRaw) chat, err := db.InsertChat(dbauthz.AsSystemRestricted(ctx), database.InsertChatParams{ @@ -10429,7 +10451,7 @@ func TestChatDebugRuns(t *testing.T) { seedChatDebugRun(ctx, t, db, chat.ID, time.Now().UTC()) - otherClientRaw, _ := coderdtest.CreateAnotherUser(t, client.Client, firstUser.OrganizationID, rbac.RoleAgentsAccess()) + otherClientRaw, _ := coderdtest.CreateAnotherUser(t, client.Client, firstUser.OrganizationID, rbac.ScopedRoleAgentsAccess(firstUser.OrganizationID)) otherClient := codersdk.NewExperimentalClient(otherClientRaw) _, err = otherClient.GetChatDebugRuns(ctx, chat.ID) @@ -11427,7 +11449,7 @@ func TestSubmitToolResults(t *testing.T) { // to user A's chat. otherClientRaw, _ := coderdtest.CreateAnotherUser( t, client.Client, user.OrganizationID, - rbac.RoleAgentsAccess(), + rbac.ScopedRoleAgentsAccess(user.OrganizationID), ) otherClient := codersdk.NewExperimentalClient(otherClientRaw) @@ -11447,9 +11469,10 @@ func TestSubmitToolResults(t *testing.T) { firstUser := coderdtest.CreateFirstUser(t, client.Client) modelConfig := createChatModelConfig(t, client) - // Create a member without agents-access. Even though the - // member owns the chat, submitting tool results should be - // gated by agents-access because it triggers inference. + // Create a member without agents-access. Without + // agents-access the member has no ResourceChat + // permissions, so the ChatParam middleware returns 404 + // before the handler can check agents-access. memberClientRaw, member := coderdtest.CreateAnotherUser(t, client.Client, firstUser.OrganizationID) memberClient := codersdk.NewExperimentalClient(memberClientRaw) @@ -11463,7 +11486,7 @@ func TestSubmitToolResults(t *testing.T) { {ToolCallID: "call_noaccess", Output: json.RawMessage(`"should fail"`)}, }, }) - requireSDKError(t, err, http.StatusForbidden) + requireSDKError(t, err, http.StatusNotFound) }) t.Run("ArchivedChat", func(t *testing.T) { diff --git a/coderd/rbac/roles.go b/coderd/rbac/roles.go index f5805bf7fa..1a97fa594f 100644 --- a/coderd/rbac/roles.go +++ b/coderd/rbac/roles.go @@ -143,7 +143,7 @@ func RoleTemplateAdmin() RoleIdentifier { return RoleIdentifier{Name: templateAd func RoleUserAdmin() RoleIdentifier { return RoleIdentifier{Name: userAdmin} } func RoleMember() RoleIdentifier { return RoleIdentifier{Name: member} } func RoleAuditor() RoleIdentifier { return RoleIdentifier{Name: auditor} } -func RoleAgentsAccess() RoleIdentifier { return RoleIdentifier{Name: agentsAccess} } +func RoleAgentsAccess() string { return agentsAccess } func RoleOrgAdmin() string { return orgAdmin @@ -199,6 +199,10 @@ func ScopedRoleOrgWorkspaceCreationBan(organizationID uuid.UUID) RoleIdentifier return RoleIdentifier{Name: RoleOrgWorkspaceCreationBan(), OrganizationID: organizationID} } +func ScopedRoleAgentsAccess(organizationID uuid.UUID) RoleIdentifier { + return RoleIdentifier{Name: RoleAgentsAccess(), OrganizationID: organizationID} +} + func allPermsExcept(excepts ...Objecter) []Permission { resources := AllResources() var perms []Permission @@ -404,21 +408,6 @@ func ReloadBuiltinRoles(opts *RoleOptions) { ByOrgID: map[string]OrgPermissions{}, }.withCachedRegoValue() - agentsAccessRole := Role{ - Identifier: RoleAgentsAccess(), - DisplayName: "Coder Agents User", - Site: []Permission{}, - User: Permissions(map[string][]policy.Action{ - ResourceChat.Type: { - policy.ActionCreate, - policy.ActionRead, - policy.ActionUpdate, - policy.ActionDelete, - }, - }), - ByOrgID: map[string]OrgPermissions{}, - }.withCachedRegoValue() - builtInRoles = map[string]func(orgID uuid.UUID) Role{ // admin grants all actions to all resources. owner: func(_ uuid.UUID) Role { @@ -449,14 +438,6 @@ func ReloadBuiltinRoles(opts *RoleOptions) { return userAdminRole }, - // agentsAccess grants all actions on chat resources owned - // by the user. Without this role, members can still read, - // update, and delete their own chats via org membership, - // but cannot create chats or trigger AI inference. - agentsAccess: func(_ uuid.UUID) Role { - return agentsAccessRole - }, - // orgAdmin returns a role with all actions allows in a given // organization scope. orgAdmin: func(organizationID uuid.UUID) Role { @@ -606,6 +587,30 @@ func ReloadBuiltinRoles(opts *RoleOptions) { }, } }, + // agentsAccess grants org members permission to create, read, and + // update chats. ActionDelete is intentionally excluded: no dbauthz + // function checks it on ResourceChat. Hard-deletion goes through + // ResourceSystem (dbpurge). + agentsAccess: func(organizationID uuid.UUID) Role { + return Role{ + Identifier: RoleIdentifier{Name: agentsAccess, OrganizationID: organizationID}, + DisplayName: "Coder Agents User", + Site: []Permission{}, + User: []Permission{}, + ByOrgID: map[string]OrgPermissions{ + organizationID.String(): { + Org: []Permission{}, + Member: Permissions(map[string][]policy.Action{ + ResourceChat.Type: { + policy.ActionCreate, + policy.ActionRead, + policy.ActionUpdate, + }, + }), + }, + }, + } + }, } } @@ -660,10 +665,13 @@ var assignRoles = map[string]map[string]bool{ orgTemplateAdmin: true, orgWorkspaceCreationBan: true, customOrganizationRole: true, + agentsAccess: true, }, orgUserAdmin: { - orgMember: true, + orgMember: true, + agentsAccess: true, }, + prebuildsOrchestrator: { orgMember: true, }, @@ -886,20 +894,13 @@ func SiteBuiltInRoles() []Role { for _, roleF := range builtInRoles { // Must provide some non-nil uuid to filter out org roles. role := roleF(uuid.New()) - if !role.Identifier.IsOrgRole() && role.Identifier != RoleAgentsAccess() { + if !role.Identifier.IsOrgRole() { roles = append(roles, role) } } return roles } -// AgentsAccessRole returns the agents-access role for use by callers -// that need to include it conditionally (e.g. when the agents -// experiment is enabled). -func AgentsAccessRole() Role { - return builtInRoles[agentsAccess](uuid.Nil) -} - // ChangeRoleSet is a helper function that finds the difference of 2 sets of // roles. When setting a user's new roles, it is equivalent to adding and // removing roles. This set determines the changes, so that the appropriate @@ -1041,7 +1042,10 @@ func OrgMemberPermissions(org OrgSettings) OrgRolePermissions { ResourceUser, ResourceOrganizationMember, ResourceAibridgeInterception, + // Chat access requires the agents-access role. + ResourceChat, ), + Permissions(map[string][]policy.Action{ // Reduced permission set on dormant workspaces. No build, // ssh, or exec. @@ -1123,7 +1127,10 @@ func OrgServiceAccountPermissions(org OrgSettings) OrgRolePermissions { ResourceUser, ResourceOrganizationMember, ResourceAibridgeInterception, + // Chat access requires the agents-access role. + ResourceChat, ), + Permissions(map[string][]policy.Action{ // Reduced permission set on dormant workspaces. No build, // ssh, or exec. diff --git a/coderd/rbac/roles_test.go b/coderd/rbac/roles_test.go index bbd229621a..187ace42ca 100644 --- a/coderd/rbac/roles_test.go +++ b/coderd/rbac/roles_test.go @@ -49,11 +49,6 @@ func TestBuiltInRoles(t *testing.T) { require.NoError(t, r.Valid(), "invalid role") }) } - - t.Run("agents-access", func(t *testing.T) { - t.Parallel() - require.NoError(t, rbac.AgentsAccessRole().Valid(), "invalid role") - }) } // permissionGranted checks whether a permission list contains a @@ -204,7 +199,21 @@ func TestRolePermissions(t *testing.T) { orgUserAdmin := authSubject{Name: "org_user_admin", Actor: rbac.Subject{ID: templateAdminID.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.ScopedRoleOrgUserAdmin(orgID)}, Scope: rbac.ScopeAll}.WithCachedASTValue()} orgTemplateAdmin := authSubject{Name: "org_template_admin", Actor: rbac.Subject{ID: userAdminID.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.ScopedRoleOrgTemplateAdmin(orgID)}, Scope: rbac.ScopeAll}.WithCachedASTValue()} orgAdminBanWorkspace := authSubject{Name: "org_admin_workspace_ban", Actor: rbac.Subject{ID: adminID.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.ScopedRoleOrgAdmin(orgID), rbac.ScopedRoleOrgWorkspaceCreationBan(orgID)}, Scope: rbac.ScopeAll}.WithCachedASTValue()} - agentsAccessUser := authSubject{Name: "chat_access", Actor: rbac.Subject{ID: currentUser.String(), Roles: rbac.RoleIdentifiers{rbac.RoleMember(), rbac.RoleAgentsAccess()}, Scope: rbac.ScopeAll}.WithCachedASTValue()} + agentsAccessUser := func() authSubject { + memberRole, err := rbac.RoleByName(rbac.RoleMember()) + require.NoError(t, err) + agentsRole, err := rbac.RoleByName(rbac.ScopedRoleAgentsAccess(orgID)) + require.NoError(t, err) + return authSubject{ + Name: "agents_access", + Actor: rbac.Subject{ + ID: currentUser.String(), + Roles: rbac.Roles{memberRole, agentsRole}, + Scope: rbac.ScopeAll, + }.WithCachedASTValue(), + } + }() + orgMemberMe := func() authSubject { memberRole, err := rbac.RoleByName(rbac.RoleMember()) require.NoError(t, err) @@ -580,8 +589,8 @@ func TestRolePermissions(t *testing.T) { }), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, orgAdmin, templateAdmin, orgUserAdmin, orgTemplateAdmin, orgAuditor}, - false: {setOtherOrg, memberMe, agentsAccessUser, userAdmin}, + true: {owner, orgAdmin, templateAdmin, orgUserAdmin, orgTemplateAdmin, orgAuditor, agentsAccessUser}, + false: {setOtherOrg, memberMe, userAdmin}, }, }, { @@ -1096,12 +1105,21 @@ func TestRolePermissions(t *testing.T) { }, }, { - Name: "ChatUsage", - Actions: []policy.Action{policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete}, + Name: "ChatUsageCRU", + Actions: []policy.Action{policy.ActionCreate, policy.ActionRead, policy.ActionUpdate}, Resource: rbac.ResourceChat.WithID(uuid.New()).InOrg(orgID).WithOwner(currentUser.String()), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, orgAdmin, orgMemberMe}, - false: {setOtherOrg, memberMe, agentsAccessUser, userAdmin, templateAdmin, orgTemplateAdmin, orgUserAdmin, orgAuditor}, + true: {owner, orgAdmin, agentsAccessUser}, + false: {setOtherOrg, memberMe, orgMemberMe, userAdmin, templateAdmin, orgTemplateAdmin, orgUserAdmin, orgAuditor}, + }, + }, + { + Name: "ChatUsageDelete", + Actions: []policy.Action{policy.ActionDelete}, + Resource: rbac.ResourceChat.WithID(uuid.New()).InOrg(orgID).WithOwner(currentUser.String()), + AuthorizeMap: map[bool][]hasAuthSubjects{ + true: {owner, orgAdmin}, + false: {setOtherOrg, memberMe, orgMemberMe, agentsAccessUser, userAdmin, templateAdmin, orgTemplateAdmin, orgUserAdmin, orgAuditor}, }, }, } @@ -1258,6 +1276,7 @@ func TestListRoles(t *testing.T) { fmt.Sprintf("organization-user-admin:%s", orgID.String()), fmt.Sprintf("organization-template-admin:%s", orgID.String()), fmt.Sprintf("organization-workspace-creation-ban:%s", orgID.String()), + fmt.Sprintf("agents-access:%s", orgID.String()), }, orgRoleNames) } diff --git a/coderd/roles.go b/coderd/roles.go index 4779a85114..c2125363ce 100644 --- a/coderd/roles.go +++ b/coderd/roles.go @@ -5,7 +5,6 @@ import ( "github.com/google/uuid" - "github.com/coder/coder/v2/buildinfo" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/db2sdk" "github.com/coder/coder/v2/coderd/httpapi" @@ -45,12 +44,6 @@ func (api *API) AssignableSiteRoles(rw http.ResponseWriter, r *http.Request) { } siteRoles := rbac.SiteBuiltInRoles() - // Include the agents-access role only when the agents - // experiment is enabled or this is a dev build, matching - // the RequireExperimentWithDevBypass gate on chat routes. - if api.Experiments.Enabled(codersdk.ExperimentAgents) || buildinfo.IsDev() { - siteRoles = append(siteRoles, rbac.AgentsAccessRole()) - } httpapi.Write(ctx, rw, http.StatusOK, assignableRoles(actorRoles.Roles, siteRoles, dbCustomRoles)) diff --git a/enterprise/coderd/exp_chats_test.go b/enterprise/coderd/exp_chats_test.go index 94f96fa05a..342f617bc4 100644 --- a/enterprise/coderd/exp_chats_test.go +++ b/enterprise/coderd/exp_chats_test.go @@ -1137,14 +1137,13 @@ func TestCreateChatNonDefaultOrg(t *testing.T) { // Create a second (non-default) org via the API. secondOrg := coderdenttest.CreateOrganization(t, client, coderdenttest.CreateOrganizationOptions{}) - // Create a member in the default org, then add them to the second org. + // Create a member with agents-access in both orgs. memberClientRaw, member := coderdtest.CreateAnotherUser( - t, client, firstUser.OrganizationID, rbac.RoleAgentsAccess(), + t, client, firstUser.OrganizationID, + rbac.ScopedRoleAgentsAccess(firstUser.OrganizationID), + rbac.ScopedRoleAgentsAccess(secondOrg.ID), ) - _, err = client.PostOrganizationMember(ctx, secondOrg.ID, member.Username) - require.NoError(t, err) memberClient := codersdk.NewExperimentalClient(memberClientRaw) - // Create a chat in the non-default org. chat, err := memberClient.CreateChat(ctx, codersdk.CreateChatRequest{ OrganizationID: secondOrg.ID, @@ -1215,14 +1214,13 @@ func TestListChats_OrgAdminOnlySeesOwnChats(t *testing.T) { // Create a second (non-default) org. secondOrg := coderdenttest.CreateOrganization(t, client, coderdenttest.CreateOrganizationOptions{}) - // Create a regular member with agents access in the second org. - memberClientRaw, member := coderdtest.CreateAnotherUser( - t, client, firstUser.OrganizationID, rbac.RoleAgentsAccess(), + // Create a member with agents-access in both orgs. + memberClientRaw, _ := coderdtest.CreateAnotherUser( + t, client, firstUser.OrganizationID, + rbac.ScopedRoleAgentsAccess(firstUser.OrganizationID), + rbac.ScopedRoleAgentsAccess(secondOrg.ID), ) - _, err = client.PostOrganizationMember(ctx, secondOrg.ID, member.Username) - require.NoError(t, err) memberExp := codersdk.NewExperimentalClient(memberClientRaw) - // Member creates a chat in the second org. memberChat, err := memberExp.CreateChat(ctx, codersdk.CreateChatRequest{ OrganizationID: secondOrg.ID, @@ -1239,7 +1237,7 @@ func TestListChats_OrgAdminOnlySeesOwnChats(t *testing.T) { // Create an org admin in the second org with agents access. adminClientRaw, _ := coderdtest.CreateAnotherUser( t, client, firstUser.OrganizationID, - rbac.ScopedRoleOrgAdmin(secondOrg.ID), rbac.RoleAgentsAccess(), + rbac.ScopedRoleOrgAdmin(secondOrg.ID), rbac.ScopedRoleAgentsAccess(secondOrg.ID), ) adminExp := codersdk.NewExperimentalClient(adminClientRaw) diff --git a/enterprise/coderd/roles_test.go b/enterprise/coderd/roles_test.go index 40411bb5b2..7e5bc3f2f8 100644 --- a/enterprise/coderd/roles_test.go +++ b/enterprise/coderd/roles_test.go @@ -493,7 +493,6 @@ func TestListRoles(t *testing.T) { {Name: codersdk.RoleAuditor}: false, {Name: codersdk.RoleTemplateAdmin}: false, {Name: codersdk.RoleUserAdmin}: false, - {Name: codersdk.RoleAgentsAccess}: false, }), }, { @@ -507,6 +506,7 @@ func TestListRoles(t *testing.T) { {Name: codersdk.RoleOrganizationTemplateAdmin, OrganizationID: owner.OrganizationID}: false, {Name: codersdk.RoleOrganizationUserAdmin, OrganizationID: owner.OrganizationID}: false, {Name: codersdk.RoleOrganizationWorkspaceCreationBan, OrganizationID: owner.OrganizationID}: false, + {Name: codersdk.RoleAgentsAccess, OrganizationID: owner.OrganizationID}: false, }), }, { @@ -527,7 +527,6 @@ func TestListRoles(t *testing.T) { {Name: codersdk.RoleAuditor}: false, {Name: codersdk.RoleTemplateAdmin}: false, {Name: codersdk.RoleUserAdmin}: false, - {Name: codersdk.RoleAgentsAccess}: false, }), }, { @@ -541,6 +540,7 @@ func TestListRoles(t *testing.T) { {Name: codersdk.RoleOrganizationTemplateAdmin, OrganizationID: owner.OrganizationID}: true, {Name: codersdk.RoleOrganizationUserAdmin, OrganizationID: owner.OrganizationID}: true, {Name: codersdk.RoleOrganizationWorkspaceCreationBan, OrganizationID: owner.OrganizationID}: true, + {Name: codersdk.RoleAgentsAccess, OrganizationID: owner.OrganizationID}: true, }), }, { @@ -561,7 +561,6 @@ func TestListRoles(t *testing.T) { {Name: codersdk.RoleAuditor}: true, {Name: codersdk.RoleTemplateAdmin}: true, {Name: codersdk.RoleUserAdmin}: true, - {Name: codersdk.RoleAgentsAccess}: true, }), }, { @@ -575,6 +574,7 @@ func TestListRoles(t *testing.T) { {Name: codersdk.RoleOrganizationTemplateAdmin, OrganizationID: owner.OrganizationID}: true, {Name: codersdk.RoleOrganizationUserAdmin, OrganizationID: owner.OrganizationID}: true, {Name: codersdk.RoleOrganizationWorkspaceCreationBan, OrganizationID: owner.OrganizationID}: true, + {Name: codersdk.RoleAgentsAccess, OrganizationID: owner.OrganizationID}: true, }), }, } diff --git a/site/permissions.json b/site/permissions.json index 09cb75d0fb..7ec8da4087 100644 --- a/site/permissions.json +++ b/site/permissions.json @@ -120,7 +120,7 @@ "action": "read" }, "createChat": { - "object": { "resource_type": "chat", "owner_id": "me" }, + "object": { "resource_type": "chat", "any_org": true, "owner_id": "me" }, "action": "create" } } diff --git a/site/site_test.go b/site/site_test.go index 90855250b2..757e9b7997 100644 --- a/site/site_test.go +++ b/site/site_test.go @@ -102,9 +102,13 @@ func TestRenderPermissionsResolvesMe(t *testing.T) { }) require.NoError(t, err) - // GIVEN: a user with the agents-access role. - userWithRole := dbgen.User(t, db, database.User{ - RBACRoles: []string{"agents-access"}, + // GIVEN: a user with the agents-access role at the org level. + org := dbgen.Organization(t, db, database.Organization{}) + userWithRole := dbgen.User(t, db, database.User{}) + dbgen.OrganizationMember(t, db, database.OrganizationMember{ + OrganizationID: org.ID, + UserID: userWithRole.ID, + Roles: []string{rbac.RoleAgentsAccess()}, }) _, tokenWithRole := dbgen.APIKey(t, db, database.APIKey{ UserID: userWithRole.ID, @@ -119,9 +123,8 @@ func TestRenderPermissionsResolvesMe(t *testing.T) { require.Equal(t, http.StatusOK, rw.Code) // THEN: the SSR-rendered permissions include createChat = true - // because the "me" sentinel in permissions.json was resolved to - // the actor's ID, and the agents-access role grants user-scoped - // chat create permission. + // because the agents-access role grants org-scoped chat create + // permission, and the any_org check picks it up. var permsWithRole codersdk.AuthorizationResponse err = json.Unmarshal([]byte(html.UnescapeString(rw.Body.String())), &permsWithRole) require.NoError(t, err) diff --git a/site/src/pages/OrganizationSettingsPage/UserTable/EditRolesButton.stories.tsx b/site/src/pages/OrganizationSettingsPage/UserTable/EditRolesButton.stories.tsx index b3ddbcdcbd..8dc3bed218 100644 --- a/site/src/pages/OrganizationSettingsPage/UserTable/EditRolesButton.stories.tsx +++ b/site/src/pages/OrganizationSettingsPage/UserTable/EditRolesButton.stories.tsx @@ -1,6 +1,11 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; import { userEvent, within } from "storybook/test"; import { + MockAgentsAccessRole, + MockOrganizationAdminRole, + MockOrganizationAuditorRole, + MockOrganizationTemplateAdminRole, + MockOrganizationUserAdminRole, MockOwnerRole, MockSiteRoles, MockUserAdminRole, @@ -64,3 +69,20 @@ export const AdvancedOpen: Story = { await userEvent.click(canvas.getByRole("button")); }, }; + +export const OrgRoles: Story = { + args: { + selectedRoleNames: new Set([MockAgentsAccessRole.name]), + roles: [ + MockOrganizationAdminRole, + MockOrganizationUserAdminRole, + MockOrganizationTemplateAdminRole, + MockOrganizationAuditorRole, + MockAgentsAccessRole, + ], + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await userEvent.click(canvas.getByRole("button")); + }, +}; diff --git a/site/src/pages/OrganizationSettingsPage/UserTable/EditRolesButton.tsx b/site/src/pages/OrganizationSettingsPage/UserTable/EditRolesButton.tsx index f600bc114f..e2feafc3c5 100644 --- a/site/src/pages/OrganizationSettingsPage/UserTable/EditRolesButton.tsx +++ b/site/src/pages/OrganizationSettingsPage/UserTable/EditRolesButton.tsx @@ -29,7 +29,7 @@ const roleDescriptions: Record = { "user-admin": "User admin can manage all users and groups.", "template-admin": "Template admin can manage all templates and workspaces.", auditor: "Auditor can access the audit logs.", - "agents-access": "Coder Agents User allows creating and using Coder Agents.", + "agents-access": "Grants access to Coder Agents chat.", member: "Everybody is a member. This is a shared and default role for all users.", }; diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index ce9e45892c..049ae4b3cb 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -374,6 +374,16 @@ export const MockOrganizationAuditorRole: TypesGen.AssignableRoles = { organization_member_permissions: [], }; +export const MockAgentsAccessRole: TypesGen.Role = { + name: "agents-access", + display_name: "Coder Agents User", + site_permissions: [], + user_permissions: [], + organization_id: MockOrganization.id, + organization_permissions: [], + organization_member_permissions: [], +}; + export const MockRoleWithOrgPermissions: TypesGen.AssignableRoles = { name: "my-role-1", display_name: "My Role 1",