From e9cbe145b829e8a0278938115717f6b52b6a4c21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Mon, 29 Jun 2020 17:19:32 +0200 Subject: [PATCH] Store post ID when creating meeting via slash command, add some logging and set expiry time to meeting post IDs to avoid garbage in the KVStore (#134) * Store post ID when creating meeting via slash command, add some logging and set expiry time to meeting post IDs to avoid garbage in the KVStore * Fix test * Fix first meeting not being updated * Get meeting URL through OAuth * Remove unused debug line and refactor postMeeting to store the post in the helper * Fix lint * Correct error variable name, and make sure to avoid panic when err == nil and res == nil * Add client timeout and use utils.ReadAll instead of buf.ReadFrom * Add error logs and set the timeout in context * Move logs to the right place * Fix test --- server/command.go | 4 +-- server/http.go | 74 +++++++++++++++++++++++++++++++------------ server/plugin.go | 53 +++++++++++++++++++++++++++++++ server/plugin_test.go | 7 ++-- 4 files changed, 113 insertions(+), 25 deletions(-) diff --git a/server/command.go b/server/command.go index 92e39041..dd9ddb9f 100644 --- a/server/command.go +++ b/server/command.go @@ -75,8 +75,8 @@ func (p *Plugin) executeCommand(c *plugin.Context, args *model.CommandArgs) (str meetingID := zoomUser.Pmi - _, appErr = p.postMeeting(user, meetingID, args.ChannelId, "") - if appErr != nil { + err := p.postMeeting(user, meetingID, args.ChannelId, "") + if err != nil { return "Failed to post message. Please try again.", nil } return "", nil diff --git a/server/http.go b/server/http.go index 09748800..fd174828 100644 --- a/server/http.go +++ b/server/http.go @@ -152,16 +152,17 @@ func (p *Plugin) completeUserOAuthToZoom(w http.ResponseWriter, r *http.Request) OAuthToken: tok, } - if err := p.storeZoomUserInfo(zoomUserInfo); err != nil { + err = p.storeZoomUserInfo(zoomUserInfo) + if err != nil { http.Error(w, "Unable to connect user to Zoom", http.StatusInternalServerError) return } user, _ := p.API.GetUser(userID) - _, appErr = p.postMeeting(user, zoomUser.Pmi, channelID, "") - if appErr != nil { - http.Error(w, appErr.Error(), appErr.StatusCode) + err = p.postMeeting(user, zoomUser.Pmi, channelID, "") + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) return } @@ -187,18 +188,21 @@ func (p *Plugin) completeUserOAuthToZoom(w http.ResponseWriter, r *http.Request) func (p *Plugin) handleWebhook(w http.ResponseWriter, r *http.Request) { if !p.verifyWebhookSecret(r) { + p.API.LogWarn("Could not verify webhook secreet") http.Error(w, "Not authorized", http.StatusUnauthorized) return } if !strings.Contains(r.Header.Get("Content-Type"), "application/json") { res := fmt.Sprintf("Expected Content-Type 'application/json' for webhook request, received '%s'.", r.Header.Get("Content-Type")) + p.API.LogWarn(res) http.Error(w, res, http.StatusBadRequest) return } b, err := ioutil.ReadAll(r.Body) if err != nil { + p.API.LogWarn("Cannot read body from Webhook") http.Error(w, err.Error(), http.StatusBadRequest) return } @@ -230,11 +234,13 @@ func (p *Plugin) handleMeetingEnded(w http.ResponseWriter, r *http.Request, webh key := fmt.Sprintf("%v%v", postMeetingKey, webhook.Payload.Object.ID) b, appErr := p.API.KVGet(key) if appErr != nil { + p.API.LogDebug("Could not get meeting post from KVStore", "err", appErr.Error()) http.Error(w, appErr.Error(), appErr.StatusCode) return } if b == nil { + p.API.LogWarn("Stored meeting not found") http.Error(w, "Stored meeting not found", http.StatusNotFound) return } @@ -242,6 +248,7 @@ func (p *Plugin) handleMeetingEnded(w http.ResponseWriter, r *http.Request, webh postID := string(b) post, appErr := p.API.GetPost(postID) if appErr != nil { + p.API.LogWarn("Could not get meeting post by id", "err", appErr) http.Error(w, appErr.Error(), appErr.StatusCode) return } @@ -276,6 +283,7 @@ func (p *Plugin) handleMeetingEnded(w http.ResponseWriter, r *http.Request, webh _, appErr = p.API.UpdatePost(post) if appErr != nil { + p.API.LogWarn("Could not update the post", "err", appErr) http.Error(w, appErr.Error(), appErr.StatusCode) return } @@ -299,8 +307,8 @@ type startMeetingRequest struct { MeetingID int `json:"meeting_id"` } -func (p *Plugin) postMeeting(creator *model.User, meetingID int, channelID string, topic string) (*model.Post, *model.AppError) { - meetingURL := p.getMeetingURL(meetingID) +func (p *Plugin) postMeeting(creator *model.User, meetingID int, channelID string, topic string) error { + meetingURL := p.getMeetingURL(meetingID, creator.Id) if topic == "" { topic = defaultMeetingTopic } @@ -327,7 +335,17 @@ func (p *Plugin) postMeeting(creator *model.User, meetingID int, channelID strin }, } - return p.API.CreatePost(post) + createdPost, appErr := p.API.CreatePost(post) + if appErr != nil { + return appErr + } + + appErr = p.API.KVSetWithExpiry(fmt.Sprintf("%v%v", postMeetingKey, meetingID), []byte(createdPost.Id), meetingPostIDTTL) + if appErr != nil { + p.API.LogDebug("failed to store post id", "err", appErr) + } + + return nil } func (p *Plugin) handleStartMeeting(w http.ResponseWriter, r *http.Request) { @@ -384,18 +402,13 @@ func (p *Plugin) handleStartMeeting(w http.ResponseWriter, r *http.Request) { meetingID := zoomUser.Pmi - createdPost, appErr := p.postMeeting(user, meetingID, req.ChannelID, req.Topic) - if appErr != nil { - http.Error(w, appErr.Error(), appErr.StatusCode) - return - } - - if appErr = p.API.KVSet(fmt.Sprintf("%v%v", postMeetingKey, meetingID), []byte(createdPost.Id)); appErr != nil { - http.Error(w, appErr.Error(), appErr.StatusCode) + err = p.postMeeting(user, meetingID, req.ChannelID, req.Topic) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) return } - meetingURL := p.getMeetingURL(meetingID) + meetingURL := p.getMeetingURL(meetingID, userID) _, err = w.Write([]byte(fmt.Sprintf(`{"meeting_url": "%s"}`, meetingURL))) if err != nil { @@ -403,10 +416,21 @@ func (p *Plugin) handleStartMeeting(w http.ResponseWriter, r *http.Request) { } } -func (p *Plugin) getMeetingURL(meetingID int) string { - meeting, err := p.zoomClient.GetMeeting(meetingID) - if err == nil { - return meeting.JoinURL +func (p *Plugin) getMeetingURL(meetingID int, userID string) string { + if p.configuration.EnableLegacyAuth { + meeting, err := p.zoomClient.GetMeeting(meetingID) + if err == nil { + return meeting.JoinURL + } + p.API.LogDebug("failed to get meeting", "error", err.Error()) + } + + if p.configuration.EnableOAuth { + meeting, err := p.GetMeetingOAuth(meetingID, userID) + if err == nil { + return meeting.JoinURL + } + p.API.LogDebug("failed to get meeting", "error", err.Error()) } config := p.getConfiguration() @@ -419,7 +443,15 @@ func (p *Plugin) getMeetingURL(meetingID int) string { } func (p *Plugin) postConfirm(meetingID int, channelID string, topic string, userID string, creatorName string) *model.Post { - meetingURL := p.getMeetingURL(meetingID) + creator, err := p.API.GetUserByUsername(creatorName) + if err != nil { + p.API.LogDebug("error fetching user on postConfirm", "error", err.Error()) + } + creatorID := "" + if creator != nil { + creatorID = creator.Id + } + meetingURL := p.getMeetingURL(meetingID, creatorID) post := &model.Post{ UserId: p.botUserID, diff --git a/server/plugin.go b/server/plugin.go index ed02d936..0ba57ce8 100644 --- a/server/plugin.go +++ b/server/plugin.go @@ -13,6 +13,7 @@ import ( "net/http" "path/filepath" "sync" + "time" "github.com/mattermost/mattermost-server/v5/model" "github.com/mattermost/mattermost-server/v5/plugin" @@ -37,6 +38,8 @@ const ( zoomStateLength = 3 zoomOAuthMessage = "[Click here to link your Zoom account.](%s/plugins/zoom/oauth2/connect?channelID=%s)" zoomEmailMismatch = "We could not verify your Mattermost account in Zoom. Please ensure that your Mattermost email address %s matches your Zoom login email address." + + meetingPostIDTTL = 60 * 60 * 24 // One day ) type Plugin struct { @@ -314,6 +317,56 @@ func (p *Plugin) getZoomUserWithToken(token *oauth2.Token) (*zoom.User, error) { return &zoomUser, nil } +func (p *Plugin) GetMeetingOAuth(meetingID int, userID string) (*zoom.Meeting, error) { + config := p.getConfiguration() + ctx, cancelFunct := context.WithTimeout(context.Background(), time.Second*10) + defer cancelFunct() + + conf, err := p.getOAuthConfig() + if err != nil { + return nil, err + } + + zoomUserInfo, apiErr := p.getZoomUserInfo(userID) + if apiErr != nil { + return nil, apiErr + } + + client := conf.Client(ctx, zoomUserInfo.OAuthToken) + apiURL := config.ZoomAPIURL + if apiURL == "" { + apiURL = zoomDefaultAPIURL + } + + url := fmt.Sprintf("%v/meetings/%v", apiURL, meetingID) + res, err := client.Get(url) + + if err != nil { + return nil, errors.New("error fetching zoom user, err=" + err.Error()) + } + if res == nil { + return nil, errors.New("error fetching zoom user, empty result returned") + } + defer res.Body.Close() + + if res.StatusCode != http.StatusOK { + return nil, errors.New("error fetching zoom user") + } + + buf, err := ioutil.ReadAll(res.Body) + if err != nil { + return nil, errors.New("error reading response body for zoom user") + } + + var ret zoom.Meeting + + if err := json.Unmarshal(buf, &ret); err != nil { + return nil, errors.New("error unmarshalling zoom user") + } + + return &ret, nil +} + func (p *Plugin) dm(userID string, message string) error { channel, err := p.API.GetDirectChannel(userID, p.botUserID) if err != nil { diff --git a/server/plugin_test.go b/server/plugin_test.go index 736af5e6..d0695ca2 100644 --- a/server/plugin_test.go +++ b/server/plugin_test.go @@ -94,14 +94,17 @@ func TestPlugin(t *testing.T) { api.On("UpdatePost", mock.AnythingOfType("*model.Post")).Return(&model.Post{}, nil) api.On("GetPostsSince", "thechannelid", mock.AnythingOfType("int64")).Return(&model.PostList{}, nil) - api.On("KVSet", fmt.Sprintf("%v%v", postMeetingKey, 234), mock.AnythingOfType("[]uint8")).Return(nil) - api.On("KVSet", fmt.Sprintf("%v%v", postMeetingKey, 123), mock.AnythingOfType("[]uint8")).Return(nil) + api.On("KVSetWithExpiry", fmt.Sprintf("%v%v", postMeetingKey, 234), mock.AnythingOfType("[]uint8"), mock.AnythingOfType("int64")).Return(nil) + api.On("KVSetWithExpiry", fmt.Sprintf("%v%v", postMeetingKey, 123), mock.AnythingOfType("[]uint8"), mock.AnythingOfType("int64")).Return(nil) api.On("KVGet", fmt.Sprintf("%v%v", postMeetingKey, 234)).Return([]byte("thepostid"), nil) api.On("KVGet", fmt.Sprintf("%v%v", postMeetingKey, 123)).Return([]byte("thepostid"), nil) api.On("KVDelete", fmt.Sprintf("%v%v", postMeetingKey, 234)).Return(nil) + api.On("LogWarn", mock.AnythingOfType("string")).Return() + api.On("LogDebug", mock.AnythingOfType("string"), mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return() + path, err := filepath.Abs("..") require.Nil(t, err) api.On("GetBundlePath").Return(path, nil)