Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Assist] Allow removing assist conversations #26788

Merged
merged 6 commits into from
May 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions api/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -3681,6 +3681,15 @@ func (c *Client) GetAssistantMessages(ctx context.Context, req *assist.GetAssist
return messages, nil
}

// DeleteAssistantConversation deletes a conversation entry in the backend.
func (c *Client) DeleteAssistantConversation(ctx context.Context, req *assist.DeleteAssistantConversationRequest) error {
_, err := c.grpc.DeleteAssistantConversation(ctx, req)
if err != nil {
return trail.FromGRPC(err)
}
return nil
}

// IsAssistEnabled returns true if the assist is enabled or not on the auth level.
func (c *Client) IsAssistEnabled(ctx context.Context) (*assist.IsAssistEnabledResponse, error) {
resp, err := c.grpc.IsAssistEnabled(ctx, &assist.IsAssistEnabledRequest{})
Expand Down
225 changes: 156 additions & 69 deletions api/gen/proto/go/assist/v1/assist.pb.go

Large diffs are not rendered by default.

39 changes: 39 additions & 0 deletions api/gen/proto/go/assist/v1/assist_grpc.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 11 additions & 0 deletions api/proto/teleport/assist/v1/assist.proto
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,14 @@ message IsAssistEnabledResponse {
bool enabled = 1;
}

// DeleteAssistantConversationRequest is a request to delete the conversation.
message DeleteAssistantConversationRequest {
// conversationId is a unique conversation ID.
ravicious marked this conversation as resolved.
Show resolved Hide resolved
string conversation_id = 1;
// username is a username of the user who created the conversation.
string username = 2;
}

// AssistService is a service that provides an ability to communicate with the Teleport Assist.
service AssistService {
// CreateNewConversation creates a new conversation and returns the UUID of it.
Expand All @@ -121,6 +129,9 @@ service AssistService {
// GetAssistantConversations returns all conversations for the connected user.
rpc GetAssistantConversations(GetAssistantConversationsRequest) returns (GetAssistantConversationsResponse);

// DeleteAssistantConversation deletes the conversation and all messages associated with it.
rpc DeleteAssistantConversation(DeleteAssistantConversationRequest) returns (google.protobuf.Empty);

// GetAssistantMessages returns all messages associated with the given conversation ID.
rpc GetAssistantMessages(GetAssistantMessagesRequest) returns (GetAssistantMessagesResponse);

Expand Down
5 changes: 5 additions & 0 deletions lib/auth/assist/assistv1/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,11 @@ func (a *Service) GetAssistantConversations(ctx context.Context, req *assist.Get
return resp, trace.Wrap(err)
}

// DeleteAssistantConversation deletes a conversation entry and associated messages from the backend.
func (a *Service) DeleteAssistantConversation(ctx context.Context, req *assist.DeleteAssistantConversationRequest) (*emptypb.Empty, error) {
return &emptypb.Empty{}, trace.Wrap(a.backend.DeleteAssistantConversation(ctx, req))
}

// GetAssistantMessages returns all messages with given conversation ID.
func (a *Service) GetAssistantMessages(ctx context.Context, req *assist.GetAssistantMessagesRequest) (*assist.GetAssistantMessagesResponse, error) {
resp, err := a.backend.GetAssistantMessages(ctx, req)
Expand Down
5 changes: 5 additions & 0 deletions lib/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -5268,6 +5268,11 @@ func (a *Server) GetAssistantConversations(ctx context.Context, request *assist.
return resp, trace.Wrap(err)
}

// DeleteAssistantConversation deletes a conversation from the backend.
func (a *Server) DeleteAssistantConversation(ctx context.Context, request *assist.DeleteAssistantConversationRequest) error {
return trace.Wrap(a.Services.DeleteAssistantConversation(ctx, request))
}

// CompareAndSwapHeadlessAuthentication performs a compare
// and swap replacement on a headless authentication resource.
func (a *Server) CompareAndSwapHeadlessAuthentication(ctx context.Context, old, new *types.HeadlessAuthentication) (*types.HeadlessAuthentication, error) {
Expand Down
9 changes: 9 additions & 0 deletions lib/auth/auth_with_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -6127,6 +6127,15 @@ func (a *ServerWithRoles) GetAssistantMessages(ctx context.Context, req *assist.
return a.authServer.GetAssistantMessages(ctx, req)
}

// DeleteAssistantConversation deletes a conversation by ID.
func (a *ServerWithRoles) DeleteAssistantConversation(ctx context.Context, req *assist.DeleteAssistantConversationRequest) error {
if err := a.action(apidefaults.Namespace, types.KindAssistant, types.VerbDelete); err != nil {
return trace.Wrap(err)
}

return trace.Wrap(a.authServer.DeleteAssistantConversation(ctx, req))
}

// IsAssistEnabled returns true if the assist is enabled or not on the auth level.
func (a *ServerWithRoles) IsAssistEnabled(ctx context.Context) (*assist.IsAssistEnabledResponse, error) {
if err := a.action(apidefaults.Namespace, types.KindAssistant, types.VerbRead); err != nil {
Expand Down
3 changes: 3 additions & 0 deletions lib/services/assist.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ type Assistant interface {
// CreateAssistantConversation creates a new conversation entry in the backend.
CreateAssistantConversation(ctx context.Context, req *assist.CreateAssistantConversationRequest) (*assist.CreateAssistantConversationResponse, error)

// DeleteAssistantConversation deletes a conversation entry and associated messages from the backend.
DeleteAssistantConversation(ctx context.Context, req *assist.DeleteAssistantConversationRequest) error

// GetAssistantConversations returns all conversations started by a user.
GetAssistantConversations(ctx context.Context, request *assist.GetAssistantConversationsRequest) (*assist.GetAssistantConversationsResponse, error)

Expand Down
34 changes: 34 additions & 0 deletions lib/services/local/assistant.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ func (s *AssistService) CreateAssistantConversation(ctx context.Context,
return &assist.CreateAssistantConversationResponse{Id: conversationID}, nil
}

// getConversation returns a conversation from the backend.
func (s *AssistService) getConversation(ctx context.Context, username, conversationID string) (*Conversation, error) {
item, err := s.Get(ctx, backend.Key(assistantConversationPrefix, username, conversationID))
if err != nil {
Expand All @@ -104,6 +105,39 @@ func (s *AssistService) getConversation(ctx context.Context, username, conversat
return &conversation, nil
}

// DeleteAssistantConversation deletes a conversation from the backend.
func (s *AssistService) DeleteAssistantConversation(ctx context.Context, req *assist.DeleteAssistantConversationRequest) error {
if req.Username == "" {
return trace.BadParameter("missing parameter username")
}
if req.ConversationId == "" {
return trace.BadParameter("missing parameter conversation ID")
}

// Delete all messages in the conversation first, so that if the delete
// fails, the conversation is still there. Client can retry the deleting.
if err := s.deleteAllMessages(ctx, req.Username, req.ConversationId); err != nil {
return trace.Wrap(err)
}

// Delete the conversation.
if err := s.Delete(ctx, backend.Key(assistantConversationPrefix, req.Username, req.ConversationId)); err != nil {
return trace.Wrap(err)
}

return nil
}

// deleteAllMessages deletes all messages in a conversation.
func (s *AssistService) deleteAllMessages(ctx context.Context, username, conversationID string) error {
startKey := backend.Key(assistantMessagePrefix, username, conversationID)
if err := s.DeleteRange(ctx, startKey, backend.RangeEnd(startKey)); err != nil {
return trace.Wrap(err)
}

return nil
}

// UpdateAssistantConversationInfo updates the conversation title.
func (s *AssistService) UpdateAssistantConversationInfo(ctx context.Context, request *assist.UpdateAssistantConversationInfoRequest) error {
if request.ConversationId == "" {
Expand Down
18 changes: 18 additions & 0 deletions lib/services/local/assistant_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,4 +155,22 @@ func TestAssistantCRUD(t *testing.T) {
err := identity.CreateAssistantMessage(ctx, msg)
require.Error(t, err)
})

t.Run("delete conversation", func(t *testing.T) {
req := &assist.DeleteAssistantConversationRequest{
Username: username,
ConversationId: conversationID,
}
err := identity.DeleteAssistantConversation(ctx, req)
require.NoError(t, err)

reqConversations := &assist.GetAssistantConversationsRequest{
Username: username,
}

conversations, err := identity.GetAssistantConversations(ctx, reqConversations)
require.NoError(t, err)
require.Len(t, conversations.Conversations, 1)
require.NotEqual(t, conversationID, conversations.Conversations[0].Id, "conversation was not deleted")
})
}
3 changes: 3 additions & 0 deletions lib/web/apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -789,6 +789,9 @@ func (h *Handler) bindDefaultEndpoints() {
// Creates a new conversation - the conversation ID is returned in the response.
h.POST("/webapi/assistant/conversations", h.WithAuth(h.createAssistantConversation))

// Deletes the given conversation.
h.DELETE("/webapi/assistant/conversations/:conversation_id", h.WithAuth(h.deleteAssistantConversation))

// Returns all conversations for the given user.
h.GET("/webapi/assistant/conversations", h.WithAuth(h.getAssistantConversations))

Expand Down
25 changes: 25 additions & 0 deletions lib/web/assistant.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,31 @@ func (h *Handler) createAssistantConversation(_ http.ResponseWriter, r *http.Req
}, nil
}

// deleteAssistantConversation is a handler for DELETE /webapi/assistant/conversations/:conversation_id.
func (h *Handler) deleteAssistantConversation(_ http.ResponseWriter, r *http.Request,
p httprouter.Params, sctx *SessionContext,
) (any, error) {
authClient, err := sctx.GetClient()
if err != nil {
return nil, trace.Wrap(err)
}

if err := checkAssistEnabled(authClient, r.Context()); err != nil {
return nil, trace.Wrap(err)
}

conversationID := p.ByName("conversation_id")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be checked for emptiness? I assume backend will check it but checking here as well will avoid roundtrip.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is never empty. This is the part of the URL, so if empty client will get 404 as there will be no matching handler. I've added the check in a few other handlers and then removed after realizing this.


if err := authClient.DeleteAssistantConversation(r.Context(), &assistpb.DeleteAssistantConversationRequest{
ConversationId: conversationID,
Username: sctx.GetUser(),
}); err != nil {
return nil, trace.Wrap(err)
}

return OK(), nil
}

// assistantMessage is an assistant message that is sent to the client.
type assistantMessage struct {
// Type is a type of the message.
Expand Down
38 changes: 38 additions & 0 deletions web/packages/design/src/SVGIcon/Close.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
Copyright 2023 Gravitational, Inc.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

import React from 'react';

import { SVGIcon } from './SVGIcon';

import type { SVGIconProps } from './common';

export function CloseIcon({ size = 24, fill }: SVGIconProps) {
return (
<SVGIcon
viewBox="0 0 24 24"
size={size}
fill={fill}
stroke="currentColor"
strokeWidth="2"
strokeLinecap="round"
strokeLinejoin="round"
>
<path d="M18 6L6 18" />
<path d="M6 6L18 18" />
</SVGIcon>
);
}
4 changes: 3 additions & 1 deletion web/packages/design/src/SVGIcon/SVGIcon.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ export function SVGIcon({
height,
width,
fill,
}: Props) {
...svgProps
}: Props & React.SVGProps<SVGSVGElement>) {
const theme = useTheme();

return (
Expand All @@ -35,6 +36,7 @@ export function SVGIcon({
width={width || size}
height={height || size}
fill={fill || theme.colors.text.main}
{...svgProps}
>
{children}
</svg>
Expand Down
3 changes: 2 additions & 1 deletion web/packages/design/src/SVGIcon/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ export { AuditLogIcon } from './AuditLog';
export { AuthConnectorsIcon } from './AuthConnectors';
export { AWSIcon } from './AWS';
export { ChatIcon } from './Chat';
export { ChevronRightIcon } from './ChevronRight';
export { ChevronDownIcon } from './ChevronDown';
export { ChevronRightIcon } from './ChevronRight';
export { CloseIcon } from './Close';
export { DatabasesIcon } from './Databases';
export { DesktopsIcon } from './Desktops';
export { DevicesIcon } from './Devices';
Expand Down