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.

Sign up for GitHub

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

Jump to bottom

Add initial Microsoft Teams support #967

Merged
merged 7 commits into from
Mar 1, 2020
Merged

Add initial Microsoft Teams support #967

merged 7 commits into from
Mar 1, 2020

Conversation

42wim
Copy link
Owner

@42wim 42wim commented Dec 26, 2019 •
edited
Loading

This implements Microsoft Teams support.

It uses polling to check for new messages because the ChatMessagesDelta isn't working (https://github.com/microsoftgraph/microsoft-graph-docs-contrib/issues/6235)

Documentation on how to setup can be found on https://github.com/42wim/matterbridge/wiki/MS-Teams-setup

working

  • Receiving and sending messages.
  • Listening on multiple channels.
  • Messages edited in Microsoft Teams will also be edited in bridges that support message editing (mattermost, slack, discord, ..)
  • Richtext/HTML messages from Microsoft Teams will be sent as Markdown to other bridges.
  • Receiving attachments from Microsoft Teams to other bridges
  • Replies from mattermost/slack to Microsoft Teams
  • Code snippets from Microsoft teams to other bridges

not possible because Microsoft API limitations

The non-supported API issues are sending stuff to msteams, receiving stuff from msteams mostly works. Doesn't work:

  • Messages edited in other bridges also edited in MS teams
  • Sending HTML/codeblock/emoji messages to Microsoft Teams (they will not be rendered as such)
  • Sending messages with attachments and images is not supported.

codeclimate[bot]
codeclimate bot reviewed Dec 26, 2019
View reviewed changes
Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 229495 lines exceeds the maximum allowed for the inline comments feature.

codeclimate[bot]
codeclimate bot reviewed Jan 1, 2020
View reviewed changes
Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 229587 lines exceeds the maximum allowed for the inline comments feature.

codeclimate[bot]
codeclimate bot reviewed Jan 1, 2020
View reviewed changes
Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 229589 lines exceeds the maximum allowed for the inline comments feature.

@42wim 42wim added this to the 1.17.0 milestone Jan 9, 2020
codeclimate[bot]
codeclimate bot reviewed Jan 9, 2020
View reviewed changes
Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 229621 lines exceeds the maximum allowed for the inline comments feature.

mickmister
mickmister reviewed Jan 16, 2020
View reviewed changes
bridge/msteams/msteams.go
}

func (b *Bmsteams) Disconnect() error {
return nil
Copy link

Choose a reason for hiding this comment

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

Is there any cleanup to be done in the Disconnect method?

Copy link
Owner Author

Choose a reason for hiding this comment

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

No, no clean-ups needed (no state)

mickmister
mickmister reviewed Jan 16, 2020
View reviewed changes
bridge/msteams/msteams.go
}

func (b *Bmsteams) convertToMD(text string) string {
if !strings.Contains(text, "<div>") {
Copy link

Choose a reason for hiding this comment

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

Does html-generated MSTeams formatting always have a div tag in the text?

Copy link
Owner Author

Choose a reason for hiding this comment

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

According to my tests, yes

@mickmister
Copy link

Hi @42wim, excellent work!

I've pulled this branch down and followed the setup instructions here. The instructions are clear and thorough.

During setup, I had to change the following line to include the scopes that are listed in the comment:

matterbridge/bridge/msteams/msteams.go

Line 22 in a0596ab

var defaultScopes = []string{} //"openid", "profile", "offline_access", "Group.Read.All", "Group.ReadWrite.All"}

Otherwise I got this error:

[2020-01-14T22:54:15-05:00] FATAL main: Starting gateway failed: Bridge msteams.teams failed to start: 400 Bad Request: {"error":"invalid_request","error_description":"AADSTS900144: The request body must contain the following parameter: 'scope' ... }

After applying this change and setting up the connection, I've confirmed pushing messages from Mattermost to MSTeams works properly, and messages posted in MSTeams are being delivered to Mattermost.

I've verified that all of the points in the "working" section of the PR description are working properly.

Findings

Here are some findings from usage. I assume some of these are an artifact of Microsoft's API support:

Create a code block in MSTeams, and select a language for the code block. Matterbridge crashes when processing the message:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x124fc03]

goroutine 12 [running]:
github.com/42wim/matterbridge/bridge/msteams.(*Bmsteams).handleAttachments(0xc000346d50, 0xc0005639e0, 0x0, 0xc00025f890, 0x0, 0xc0003d8ba0, 0xc00025f8a0, 0xc00025f8b0, 0xc0003d8b80, 0x0, ...)
        /home/koch/go/src/github.com/mattermost/mattermost-matterbridge/bridge/msteams/handler.go:54 +0x143
github.com/42wim/matterbridge/bridge/msteams.(*Bmsteams).poll(0xc000346d50, 0xc0000d7380, 0x30)
        /home/koch/go/src/github.com/mattermost/mattermost-matterbridge/bridge/msteams/msteams.go:175 +0x79c
created by github.com/42wim/matterbridge/bridge/msteams.(*Bmsteams).JoinChannel
        /home/koch/go/src/github.com/mattermost/mattermost-matterbridge/bridge/msteams/msteams.go:75 +0x53
exit status 2

With MSTeams -> Mattermost: strikethrough and underline formatting is not rendered in Mattermost

Markdown in general seems to not work with Mattermost -> MSTeams. I can't test if "header #" or "blockquote >" work because newlines are converted to spaces.

@42wim
Copy link
Owner Author

42wim commented Jan 16, 2020

Thanks for testing, I'll look into the remaining issues!

codeclimate[bot]
codeclimate bot reviewed Jan 16, 2020
View reviewed changes
Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 229663 lines exceeds the maximum allowed for the inline comments feature.

This was referenced Feb 20, 2020
Closed
Open
codeclimate[bot]
codeclimate bot reviewed Mar 1, 2020
View reviewed changes
Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 229662 lines exceeds the maximum allowed for the inline comments feature.

@42wim 42wim changed the title WIP - Add initial Microsoft Teams support Add initial Microsoft Teams support Mar 1, 2020
codeclimate[bot]
codeclimate bot reviewed Mar 1, 2020
View reviewed changes
Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 229663 lines exceeds the maximum allowed for the inline comments feature.

@codeclimate
Copy link

codeclimate bot commented Mar 1, 2020

Code Climate has analyzed commit ff0b926 and detected 2 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 2

Too many changed lines in diff

View more on Code Climate.

@42wim 42wim merged commit 3f06a40 into master Mar 1, 2020
@42wim 42wim deleted the f-msteams branch March 21, 2020 21:35
@42wim 42wim mentioned this pull request Apr 22, 2020
Closed
@42wim 42wim mentioned this pull request Oct 1, 2020
Draft
2 tasks
@ciroiriarte
Copy link

Hello!, I'm testing integration between Telegram/WhatsApp & MS Teams but only see messages being delivered to Teams, but no messagess deliverred to Telegram/WhatsApp.

No errores in the logs, tested with 1.22.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mickmister mickmister mickmister left review comments

@codeclimate codeclimate[bot] codeclimate left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.17.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@42wim @mickmister @ciroiriarte