From 134d22d8c25043b55c0e76c801ba88e184935c3d Mon Sep 17 00:00:00 2001 From: Edward Dowling Date: Wed, 13 Nov 2024 16:56:56 +0000 Subject: [PATCH 1/2] Make msteams use common recipient with filled data field Change msteams to using the provided data field in the common recipient type to store extra required data. --- integrations/access/msteams/app.go | 8 ++- integrations/access/msteams/bot.go | 75 ++++++++++++++++--------- integrations/access/msteams/validate.go | 6 +- 3 files changed, 60 insertions(+), 29 deletions(-) diff --git a/integrations/access/msteams/app.go b/integrations/access/msteams/app.go index 306be091ca8b0..c59d1f9db9cc6 100644 --- a/integrations/access/msteams/app.go +++ b/integrations/access/msteams/app.go @@ -185,7 +185,11 @@ func (a *App) initBot(ctx context.Context) error { a.log.InfoContext(ctx, "Preloading recipient data...") for _, recipient := range a.conf.Recipients.GetAllRawRecipients() { - recipientData, err := a.bot.FetchRecipient(ctx, recipient) + fullRecipient, err := a.bot.FetchRecipient(ctx, recipient) + if err != nil { + return trace.Wrap(err) + } + recipientData, err := getRecipientData(fullRecipient) if err != nil { return trace.Wrap(err) } @@ -193,7 +197,7 @@ func (a *App) initBot(ctx context.Context) error { slog.Group("recipient", "raw", recipient, "recipient_chat_id", recipientData.Chat.ID, - "recipient_kind", recipientData.Kind, + "recipient_kind", fullRecipient.Kind, ), ) } diff --git a/integrations/access/msteams/bot.go b/integrations/access/msteams/bot.go index c0598c1f4d24f..f9f4ef3e7dfa7 100644 --- a/integrations/access/msteams/bot.go +++ b/integrations/access/msteams/bot.go @@ -41,16 +41,12 @@ const ( type RecipientKind string -// RecipientData represents cached data for a recipient (user or channel) -type RecipientData struct { - // ID identifies the recipient, for users it is the UserID, for channels it is "tenant/group/channelName" - ID string +// recipientData represents cached data for a recipient (user or channel) +type recipientData struct { // App installation for the recipient App msapi.InstalledApp // Chat for the recipient Chat msapi.Chat - // Kind of the recipient (user or channel) - Kind RecipientKind } // Channel represents a MSTeams channel parsed from its web URL @@ -75,7 +71,7 @@ type Bot struct { // mu recipients access mutex mu *sync.RWMutex // recipients represents the cache of potential message recipients - recipients map[string]RecipientData + recipients map[string]common.Recipient // webProxyURL represents Web UI address, if enabled webProxyURL *url.URL // clusterName cluster name @@ -105,7 +101,7 @@ func NewBot(c *Config, clusterName, webProxyAddr string, log *slog.Logger) (*Bot Config: c.MSAPI, graphClient: msapi.NewGraphClient(c.MSAPI), botClient: msapi.NewBotFrameworkClient(c.MSAPI), - recipients: make(map[string]RecipientData), + recipients: make(map[string]common.Recipient), webProxyURL: webProxyURL, clusterName: clusterName, mu: &sync.RWMutex{}, @@ -181,7 +177,7 @@ func (b *Bot) UninstallAppForUser(ctx context.Context, userIDOrEmail string) err // FetchRecipient checks if recipient is a user or a channel, installs app for a user if missing, fetches chat id // and saves everything to cache. This method is used for priming the cache. Returns trace.NotFound if a // user was not found. -func (b *Bot) FetchRecipient(ctx context.Context, recipient string) (*RecipientData, error) { +func (b *Bot) FetchRecipient(ctx context.Context, recipient string) (*common.Recipient, error) { if b.teamsApp == nil { return nil, trace.Errorf("Bot is not configured, run GetTeamsApp first") } @@ -192,14 +188,18 @@ func (b *Bot) FetchRecipient(ctx context.Context, recipient string) (*RecipientD b.mu.RLock() d, ok := b.recipients[recipient] b.mu.RUnlock() + rd, err := getRecipientData(&d) + if err != nil { + return nil, trace.Wrap(err) + } if ok { log.DebugContext(ctx, "Found recipient in cache", slog.Group("recipient", "id", d.ID, - "installed_app_id", d.App.ID, - "chat_id", d.Chat.ID, - "chat_url", d.Chat.WebURL, - "chat_tenant_id", d.Chat.TenantID, + "installed_app_id", rd.App.ID, + "chat_id", rd.Chat.ID, + "chat_url", rd.Chat.WebURL, + "chat_tenant_id", rd.Chat.TenantID, "kind", d.Kind, ), ) @@ -220,7 +220,6 @@ func (b *Bot) FetchRecipient(ctx context.Context, recipient string) (*RecipientD ), ) // A team and a group are different but in MsTeams the team is associated to a group and will have the same id. - log = log.With("teams_app_id", b.teamsApp.ID, "channel_group", channel.Group) log.DebugContext(ctx, "Retrieving the app installation ID for the team") installedApp, err := b.graphClient.GetAppForTeam(ctx, b.teamsApp, channel.Group) @@ -228,16 +227,20 @@ func (b *Bot) FetchRecipient(ctx context.Context, recipient string) (*RecipientD log.ErrorContext(ctx, "Failed to retrieve the app installation ID for the team", "error", err) return nil, trace.Wrap(err) } - d = RecipientData{ - ID: fmt.Sprintf("%s/%s/%s", channel.Tenant, channel.Group, channel.Name), - App: *installedApp, - Chat: msapi.Chat{ - ID: channel.ChatID, - TenantID: channel.Tenant, - WebURL: channel.URL.String(), + d = common.Recipient{ + Name: recipient, + ID: fmt.Sprintf("%s/%s/%s", channel.Tenant, channel.Group, channel.Name), + Kind: string(RecipientKindChannel), + Data: recipientData{ + App: *installedApp, + Chat: msapi.Chat{ + ID: channel.ChatID, + TenantID: channel.Tenant, + WebURL: channel.URL.String(), + }, }, - Kind: RecipientKindChannel, } + log.DebugContext(ctx, "Retrieved the app installation ID for the team", "recipient_installed_app_id", installedApp.ID) // If the recipient is not a channel, it means it is a user (either email or userID) @@ -246,7 +249,7 @@ func (b *Bot) FetchRecipient(ctx context.Context, recipient string) (*RecipientD userID, err := b.getUserID(ctx, recipient) if err != nil { log.ErrorContext(ctx, "Failed to resolve recipient", "error", err) - return &RecipientData{}, trace.Wrap(err) + return &common.Recipient{}, trace.Wrap(err) } log = log.With("user_id", userID) log.DebugContext(ctx, "Successfully resolve user recipient") @@ -285,7 +288,15 @@ func (b *Bot) FetchRecipient(ctx context.Context, recipient string) (*RecipientD } log.DebugContext(ctx, "Found the chat ID for the user", "chat_id", chat.ID) - d = RecipientData{userID, *installedApp, chat, RecipientKindUser} + d = common.Recipient{ + Name: recipient, + ID: userID, + Kind: string(RecipientKindUser), + Data: recipientData{ + App: *installedApp, + Chat: chat, + }, + } } b.mu.Lock() @@ -316,8 +327,12 @@ func (b *Bot) getUserID(ctx context.Context, userIDOrEmail string) (string, erro } // PostAdaptiveCardActivity sends the AdaptiveCard to a user -func (b *Bot) PostAdaptiveCardActivity(ctx context.Context, recipient, cardBody, updateID string) (string, error) { - recipientData, err := b.FetchRecipient(ctx, recipient) +func (b *Bot) PostAdaptiveCardActivity(ctx context.Context, recipient string, cardBody, updateID string) (string, error) { + fullRecipient, err := b.FetchRecipient(ctx, recipient) + if err != nil { + return "", trace.Wrap(err) + } + recipientData, err := getRecipientData(fullRecipient) if err != nil { return "", trace.Wrap(err) } @@ -474,3 +489,11 @@ func (b *Bot) CheckHealth(ctx context.Context) error { } return trace.Wrap(err) } + +func getRecipientData(recipient *common.Recipient) (recipientData, error) { + rd, ok := recipient.Data.(recipientData) + if !ok { + return recipientData{}, trace.BadParameter("unexpected recipient data type") + } + return rd, nil +} diff --git a/integrations/access/msteams/validate.go b/integrations/access/msteams/validate.go index 61d9d25f635e8..20d27361c31e5 100644 --- a/integrations/access/msteams/validate.go +++ b/integrations/access/msteams/validate.go @@ -56,7 +56,11 @@ func Validate(configPath, recipient string) error { recipient = userID } - recipientData, err := b.FetchRecipient(context.Background(), recipient) + fullRecipient, err := b.FetchRecipient(ctx, recipient) + if err != nil { + return trace.Wrap(err) + } + recipientData, err := getRecipientData(fullRecipient) if err != nil { return trace.Wrap(err) } From f1c892fcba17b98703e4f58a8098e71dba9efb7a Mon Sep 17 00:00:00 2001 From: Edward Dowling Date: Fri, 15 Nov 2024 18:08:49 +0000 Subject: [PATCH 2/2] Remove unsed func and rename recipients in msteams bot --- integrations/access/common/recipient.go | 9 --------- integrations/access/msteams/bot.go | 10 +++++----- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/integrations/access/common/recipient.go b/integrations/access/common/recipient.go index 0fbe218bcd1fe..333a197206b48 100644 --- a/integrations/access/common/recipient.go +++ b/integrations/access/common/recipient.go @@ -150,15 +150,6 @@ func (s *RecipientSet) ToSlice() []Recipient { return recipientSlice } -// GetNames returns a slice of the recipient names in the set. -func (s *RecipientSet) GetNames() []string { - names := make([]string, 0, len(s.recipients)) - for _, recipient := range s.recipients { - names = append(names, recipient.Name) - } - return names -} - // ForEach applies run the given func with each recipient in the set as the argument. func (s *RecipientSet) ForEach(f func(r Recipient)) { for _, v := range s.recipients { diff --git a/integrations/access/msteams/bot.go b/integrations/access/msteams/bot.go index f9f4ef3e7dfa7..ca89fa366ae1e 100644 --- a/integrations/access/msteams/bot.go +++ b/integrations/access/msteams/bot.go @@ -70,8 +70,8 @@ type Bot struct { botClient *msapi.BotFrameworkClient // mu recipients access mutex mu *sync.RWMutex - // recipients represents the cache of potential message recipients - recipients map[string]common.Recipient + // fullRecipients represents the cache of potential message fullRecipients + fullRecipients map[string]common.Recipient // webProxyURL represents Web UI address, if enabled webProxyURL *url.URL // clusterName cluster name @@ -101,7 +101,7 @@ func NewBot(c *Config, clusterName, webProxyAddr string, log *slog.Logger) (*Bot Config: c.MSAPI, graphClient: msapi.NewGraphClient(c.MSAPI), botClient: msapi.NewBotFrameworkClient(c.MSAPI), - recipients: make(map[string]common.Recipient), + fullRecipients: make(map[string]common.Recipient), webProxyURL: webProxyURL, clusterName: clusterName, mu: &sync.RWMutex{}, @@ -186,7 +186,7 @@ func (b *Bot) FetchRecipient(ctx context.Context, recipient string) (*common.Rec log.DebugContext(ctx, "Fetching recipient") b.mu.RLock() - d, ok := b.recipients[recipient] + d, ok := b.fullRecipients[recipient] b.mu.RUnlock() rd, err := getRecipientData(&d) if err != nil { @@ -300,7 +300,7 @@ func (b *Bot) FetchRecipient(ctx context.Context, recipient string) (*common.Rec } b.mu.Lock() - b.recipients[recipient] = d + b.fullRecipients[recipient] = d b.mu.Unlock() return &d, nil