opensource/coder
3
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2026-09-24 15:04:27 +08:00
Code Issues Packages Projects Releases Wiki Activity

feat: add chat sharing foundation (#25041)

Browse Source
This commit is contained in:
Danielle Maywood
2026-05-18 22:32:05 +01:00
committed by GitHub
parent 2732378da2
commit 170a6e1fe9
49 changed files with 1872 additions and 103 deletions
Download Patch File Download Diff File Expand all files Collapse all files
coderd/database/dbauthz/dbauthz.go
+71 -3
View File
@@ -2682,6 +2682,17 @@ func (q *querier) GetAuthorizationUserRoles(ctx context.Context, userID uuid.UUI
return q.db.GetAuthorizationUserRoles(ctx, userID)
}
func (q *querier) GetChatACLByID(ctx context.Context, id uuid.UUID) (database.GetChatACLByIDRow, error) {
chat, err := q.db.GetChatByID(ctx, id)
if err != nil {
return database.GetChatACLByIDRow{}, err
}
if err := q.authorizeContext(ctx, policy.ActionRead, chat); err != nil {
return database.GetChatACLByIDRow{}, err
}
return q.db.GetChatACLByID(ctx, id)
}
func (q *querier) GetChatAdvisorConfig(ctx context.Context) (string, error) {
// The advisor configuration is a deployment-wide setting read by any
// authenticated chat user and by chatd when deciding whether to attach
@@ -2884,14 +2895,30 @@ func (q *querier) GetChatFileByID(ctx context.Context, id uuid.UUID) (database.C
if err != nil {
return database.ChatFile{}, err
}
if err := q.authorizeContext(ctx, policy.ActionRead, file); err != nil {
fileAuthErr := q.authorizeContext(ctx, policy.ActionRead, file)
if fileAuthErr == nil {
return file, nil
}
prepared, err := prepareSQLFilter(ctx, q.auth, policy.ActionRead, rbac.ResourceChat.Type)
if err != nil {
return database.ChatFile{}, xerrors.Errorf("(dev error) prepare sql filter: %w", err)
}
chats, err := q.db.GetAuthorizedChatsByChatFileID(ctx, id, prepared)
if err != nil {
return database.ChatFile{}, err
}
if len(chats) == 0 {
return database.ChatFile{}, fileAuthErr
}
return file, nil
}
func (q *querier) GetChatFileMetadataByChatID(ctx context.Context, chatID uuid.UUID) ([]database.GetChatFileMetadataByChatIDRow, error) {
return fetchWithPostFilter(q.auth, policy.ActionRead, q.db.GetChatFileMetadataByChatID)(ctx, chatID)
if _, err := q.GetChatByID(ctx, chatID); err != nil {
return nil, err
}
return q.db.GetChatFileMetadataByChatID(ctx, chatID)
}
func (q *querier) GetChatFilesByIDs(ctx context.Context, ids []uuid.UUID) ([]database.ChatFile, error) {
@@ -2899,10 +2926,25 @@ func (q *querier) GetChatFilesByIDs(ctx context.Context, ids []uuid.UUID) ([]dat
if err != nil {
return nil, err
}
var prepared rbac.PreparedAuthorized
for _, f := range files {
if err := q.authorizeContext(ctx, policy.ActionRead, f); err != nil {
fileAuthErr := q.authorizeContext(ctx, policy.ActionRead, f)
if fileAuthErr == nil {
continue
}
if prepared == nil {
prepared, err = prepareSQLFilter(ctx, q.auth, policy.ActionRead, rbac.ResourceChat.Type)
if err != nil {
return nil, xerrors.Errorf("(dev error) prepare sql filter: %w", err)
}
}
chats, err := q.db.GetAuthorizedChatsByChatFileID(ctx, f.ID, prepared)
if err != nil {
return nil, err
}
if len(chats) == 0 {
return nil, fileAuthErr
}
}
return files, nil
}
@@ -3164,6 +3206,10 @@ func (q *querier) GetChats(ctx context.Context, arg database.GetChatsParams) ([]
return q.db.GetAuthorizedChats(ctx, arg, prep)
}
func (q *querier) GetChatsByChatFileID(ctx context.Context, fileID uuid.UUID) ([]database.Chat, error) {
return fetchWithPostFilter(q.auth, policy.ActionRead, q.db.GetChatsByChatFileID)(ctx, fileID)
}
func (q *querier) GetChatsByWorkspaceIDs(ctx context.Context, ids []uuid.UUID) ([]database.Chat, error) {
return fetchWithPostFilter(q.auth, policy.ActionRead, q.db.GetChatsByWorkspaceIDs)(ctx, ids)
}
@@ -6392,6 +6438,24 @@ func (q *querier) UpdateAPIKeyByID(ctx context.Context, arg database.UpdateAPIKe
return update(q.log, q.auth, fetch, q.db.UpdateAPIKeyByID)(ctx, arg)
}
func (q *querier) UpdateChatACLByID(ctx context.Context, arg database.UpdateChatACLByIDParams) error {
if rbac.ChatACLDisabled() {
return NotAuthorizedError{Err: xerrors.New("chat sharing is disabled")}
}
fetch := func(ctx context.Context, arg database.UpdateChatACLByIDParams) (database.Chat, error) {
chat, err := q.db.GetChatByID(ctx, arg.ID)
if err != nil {
return database.Chat{}, err
}
if chat.IsSubChat() {
return database.Chat{}, NotAuthorizedError{Err: xerrors.New("chat ACLs can only be updated on root chats")}
}
return chat, nil
}
return fetchAndExec(q.log, q.auth, policy.ActionShare, fetch, q.db.UpdateChatACLByID)(ctx, arg)
}
func (q *querier) UpdateChatBuildAgentBinding(ctx context.Context, arg database.UpdateChatBuildAgentBindingParams) (database.Chat, error) {
chat, err := q.db.GetChatByID(ctx, arg.ID)
if err != nil {
@@ -8323,3 +8387,7 @@ func (q *querier) ListAuthorizedAIBridgeSessionThreads(ctx context.Context, arg
func (q *querier) GetAuthorizedChats(ctx context.Context, arg database.GetChatsParams, _ rbac.PreparedAuthorized) ([]database.GetChatsRow, error) {
return q.GetChats(ctx, arg)
}
func (q *querier) GetAuthorizedChatsByChatFileID(ctx context.Context, fileID uuid.UUID, prepared rbac.PreparedAuthorized) ([]database.Chat, error) {
return q.db.GetAuthorizedChatsByChatFileID(ctx, fileID, prepared)
}
coderd/database/dbauthz/dbauthz_test.go
+149 -2
View File
@@ -155,6 +155,108 @@ func TestNew(t *testing.T) {
require.NoError(t, rec.AllAsserted(), "should only be 1 rbac call")
}
func TestChatFilesAllowLinkedChatReads(t *testing.T) {
t.Parallel()
ctx := dbauthz.As(context.Background(), rbac.Subject{
ID: uuid.NewString(),
Scope: rbac.ScopeAll,
})
authorizer := &coderdtest.FakeAuthorizer{
ConditionalReturn: func(_ context.Context, _ rbac.Subject, action policy.Action, object rbac.Object) error {
if action == policy.ActionRead && object.Type == rbac.ResourceChat.Type {
return xerrors.New("direct file auth denied")
}
return nil
},
}
t.Run("GetChatFileByID", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
db := dbmock.NewMockStore(ctrl)
file := testutil.Fake(t, gofakeit.New(0), database.ChatFile{})
db.EXPECT().Wrappers().Return([]string{}).AnyTimes()
db.EXPECT().GetChatFileByID(gomock.Any(), file.ID).Return(file, nil)
db.EXPECT().GetAuthorizedChatsByChatFileID(gomock.Any(), file.ID, gomock.Any()).Return([]database.Chat{{ID: uuid.New()}}, nil)
q := dbauthz.New(db, authorizer, slogtest.Make(t, nil), coderdtest.AccessControlStorePointer())
got, err := q.GetChatFileByID(ctx, file.ID)
require.NoError(t, err)
require.Equal(t, file, got)
})
t.Run("GetChatFilesByIDs", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
db := dbmock.NewMockStore(ctrl)
file := testutil.Fake(t, gofakeit.New(0), database.ChatFile{})
db.EXPECT().Wrappers().Return([]string{}).AnyTimes()
db.EXPECT().GetChatFilesByIDs(gomock.Any(), []uuid.UUID{file.ID}).Return([]database.ChatFile{file}, nil)
db.EXPECT().GetAuthorizedChatsByChatFileID(gomock.Any(), file.ID, gomock.Any()).Return([]database.Chat{{ID: uuid.New()}}, nil)
q := dbauthz.New(db, authorizer, slogtest.Make(t, nil), coderdtest.AccessControlStorePointer())
got, err := q.GetChatFilesByIDs(ctx, []uuid.UUID{file.ID})
require.NoError(t, err)
require.Equal(t, []database.ChatFile{file}, got)
})
}
//nolint:tparallel,paralleltest // It toggles the global chat ACL flag.
func TestUpdateChatACLByIDGuards(t *testing.T) {
ctx := dbauthz.As(context.Background(), rbac.Subject{
ID: uuid.NewString(),
Scope: rbac.ScopeAll,
})
arg := database.UpdateChatACLByIDParams{
ID: uuid.New(),
UserACL: database.ChatACL{},
GroupACL: database.ChatACL{},
}
t.Run("Disabled", func(t *testing.T) { //nolint:paralleltest // It toggles the global chat ACL flag.
rbac.SetChatACLDisabled(true)
t.Cleanup(func() { rbac.SetChatACLDisabled(false) })
ctrl := gomock.NewController(t)
db := dbmock.NewMockStore(ctrl)
db.EXPECT().Wrappers().Return([]string{}).AnyTimes()
q := dbauthz.New(db, &coderdtest.FakeAuthorizer{}, slogtest.Make(t, nil), coderdtest.AccessControlStorePointer())
err := q.UpdateChatACLByID(ctx, arg)
require.Error(t, err)
require.True(t, dbauthz.IsNotAuthorizedError(err))
require.ErrorContains(t, err, "chat sharing is disabled")
})
t.Run("SubChat", func(t *testing.T) { //nolint:paralleltest // It depends on the global chat ACL flag.
rbac.SetChatACLDisabled(false)
ctrl := gomock.NewController(t)
db := dbmock.NewMockStore(ctrl)
db.EXPECT().Wrappers().Return([]string{}).AnyTimes()
db.EXPECT().GetChatByID(gomock.Any(), arg.ID).Return(database.Chat{
ID: arg.ID,
RootChatID: uuid.NullUUID{UUID: uuid.New(), Valid: true},
ParentChatID: uuid.NullUUID{UUID: uuid.New(), Valid: true},
}, nil)
q := dbauthz.New(db, &coderdtest.FakeAuthorizer{}, slogtest.Make(t, nil), coderdtest.AccessControlStorePointer())
err := q.UpdateChatACLByID(ctx, arg)
require.Error(t, err)
require.True(t, dbauthz.IsNotAuthorizedError(err))
require.ErrorContains(t, err, "root chats")
})
}
// TestDBAuthzRecursive is a simple test to search for infinite recursion
// bugs. It isn't perfect, and only catches a subset of the possible bugs
// as only the first db call will be made. But it is better than nothing.
@@ -585,6 +687,20 @@ func (s *MethodTestSuite) TestChats() {
dbm.EXPECT().UpsertChatPersonalModelOverridesEnabled(gomock.Any(), true).Return(nil).AnyTimes()
check.Args(true).Asserts(rbac.ResourceDeploymentConfig, policy.ActionUpdate)
}))
s.Run("GetChatACLByID", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) {
chat := testutil.Fake(s.T(), faker, database.Chat{})
row := database.GetChatACLByIDRow{
Users: database.ChatACL{
uuid.NewString(): database.ChatACLEntry{Permissions: []policy.Action{policy.ActionRead}},
},
Groups: database.ChatACL{
uuid.NewString(): database.ChatACLEntry{Permissions: []policy.Action{policy.ActionRead}},
},
}
dbm.EXPECT().GetChatByID(gomock.Any(), chat.ID).Return(chat, nil).AnyTimes()
dbm.EXPECT().GetChatACLByID(gomock.Any(), chat.ID).Return(row, nil).AnyTimes()
check.Args(chat.ID).Asserts(chat, policy.ActionRead).Returns(row)
}))
s.Run("GetChatByID", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) {
chat := testutil.Fake(s.T(), faker, database.Chat{})
dbm.EXPECT().GetChatByID(gomock.Any(), chat.ID).Return(chat, nil).AnyTimes()
@@ -722,14 +838,17 @@ func (s *MethodTestSuite) TestChats() {
s.Run("GetChatFileByID", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) {
file := testutil.Fake(s.T(), faker, database.ChatFile{})
dbm.EXPECT().GetChatFileByID(gomock.Any(), file.ID).Return(file, nil).AnyTimes()
dbm.EXPECT().GetAuthorizedChatsByChatFileID(gomock.Any(), file.ID, gomock.Any()).Return([]database.Chat{}, nil).AnyTimes()
check.Args(file.ID).Asserts(rbac.ResourceChat.WithOwner(file.OwnerID.String()).InOrg(file.OrganizationID).WithID(file.ID), policy.ActionRead).Returns(file)
}))
s.Run("GetChatFilesByIDs", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) {
file := testutil.Fake(s.T(), faker, database.ChatFile{})
dbm.EXPECT().GetChatFilesByIDs(gomock.Any(), []uuid.UUID{file.ID}).Return([]database.ChatFile{file}, nil).AnyTimes()
dbm.EXPECT().GetAuthorizedChatsByChatFileID(gomock.Any(), file.ID, gomock.Any()).Return([]database.Chat{}, nil).AnyTimes()
check.Args([]uuid.UUID{file.ID}).Asserts(rbac.ResourceChat.WithOwner(file.OwnerID.String()).InOrg(file.OrganizationID).WithID(file.ID), policy.ActionRead).Returns([]database.ChatFile{file})
}))
s.Run("GetChatFileMetadataByChatID", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) {
chat := testutil.Fake(s.T(), faker, database.Chat{})
file := testutil.Fake(s.T(), faker, database.ChatFile{})
rows := []database.GetChatFileMetadataByChatIDRow{{
ID: file.ID,
@@ -739,8 +858,9 @@ func (s *MethodTestSuite) TestChats() {
OwnerID: file.OwnerID,
OrganizationID: file.OrganizationID,
}}
dbm.EXPECT().GetChatFileMetadataByChatID(gomock.Any(), file.ID).Return(rows, nil).AnyTimes()
check.Args(file.ID).Asserts(rbac.ResourceChat.WithOwner(file.OwnerID.String()).InOrg(file.OrganizationID).WithID(file.ID), policy.ActionRead).Returns(rows)
dbm.EXPECT().GetChatByID(gomock.Any(), chat.ID).Return(chat, nil).AnyTimes()
dbm.EXPECT().GetChatFileMetadataByChatID(gomock.Any(), chat.ID).Return(rows, nil).AnyTimes()
check.Args(chat.ID).Asserts(chat, policy.ActionRead).Returns(rows)
}))
s.Run("DeleteOldChatDebugRuns", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) {
dbm.EXPECT().DeleteOldChatDebugRuns(gomock.Any(), database.DeleteOldChatDebugRunsParams{}).Return(int64(0), nil).AnyTimes()
@@ -886,6 +1006,14 @@ func (s *MethodTestSuite) TestChats() {
// No asserts here because SQLFilter.
check.Args(params).Asserts()
}))
s.Run("GetChatsByChatFileID", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) {
chatA := testutil.Fake(s.T(), faker, database.Chat{})
chatB := testutil.Fake(s.T(), faker, database.Chat{})
fileID := uuid.New()
chats := []database.Chat{chatA, chatB}
dbm.EXPECT().GetChatsByChatFileID(gomock.Any(), fileID).Return(chats, nil).AnyTimes()
check.Args(fileID).Asserts(chatA, policy.ActionRead, chatB, policy.ActionRead).Returns(chats)
}))
s.Run("GetChildChatsByParentIDs", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) {
parentA := testutil.Fake(s.T(), faker, database.Chat{})
parentB := testutil.Fake(s.T(), faker, database.Chat{})
@@ -913,6 +1041,12 @@ func (s *MethodTestSuite) TestChats() {
// No asserts here because it re-routes through GetChats which uses SQLFilter.
check.Args(params, emptyPreparedAuthorized{}).Asserts()
}))
s.Run("GetAuthorizedChatsByChatFileID", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) {
fileID := uuid.New()
dbm.EXPECT().GetAuthorizedChatsByChatFileID(gomock.Any(), fileID, gomock.Any()).Return([]database.Chat{}, nil).AnyTimes()
// No asserts here because callers provide the SQL filter.
check.Args(fileID, emptyPreparedAuthorized{}).Asserts()
}))
s.Run("GetChatQueuedMessages", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) {
chat := testutil.Fake(s.T(), faker, database.Chat{})
qms := []database.ChatQueuedMessage{testutil.Fake(s.T(), faker, database.ChatQueuedMessage{})}
@@ -1057,6 +1191,19 @@ func (s *MethodTestSuite) TestChats() {
dbm.EXPECT().ReorderChatQueuedMessageToFront(gomock.Any(), arg).Return(int64(1), nil).AnyTimes()
check.Args(arg).Asserts(chat, policy.ActionUpdate).Returns(int64(1))
}))
s.Run("UpdateChatACLByID", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) {
chat := testutil.Fake(s.T(), faker, database.Chat{})
chat.RootChatID = uuid.NullUUID{}
chat.ParentChatID = uuid.NullUUID{}
arg := database.UpdateChatACLByIDParams{
ID: chat.ID,
UserACL: database.ChatACL{},
GroupACL: database.ChatACL{},
}
dbm.EXPECT().GetChatByID(gomock.Any(), chat.ID).Return(chat, nil).AnyTimes()
dbm.EXPECT().UpdateChatACLByID(gomock.Any(), arg).Return(nil).AnyTimes()
check.Args(arg).Asserts(chat, policy.ActionShare).Returns()
}))
s.Run("UpdateChatByID", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) {
chat := testutil.Fake(s.T(), faker, database.Chat{})
arg := database.UpdateChatByIDParams{