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

[MM-133] Added a feature of restricting bot messages to private channels and DM/GMs #337

Open
wants to merge 23 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
d3a7688
[MM-133] Work on zoom feature of restricting bot messages to private …
ayusht2810 Jan 17, 2024
672f29a
[MM-133] Add role on channel-settings slash command
ayusht2810 Jan 17, 2024
1fb2e67
[MM-133] Update readme and help text slash command on basis of user role
ayusht2810 Jan 17, 2024
0c51cfb
[MM-133] Fix lint issues
ayusht2810 Jan 17, 2024
fdc98fc
[MM-133] Update dialog text
ayusht2810 Jan 18, 2024
6837578
[MM-133] Review fixes: Update user statements and update if else cond…
ayusht2810 Jan 22, 2024
ecc3cf9
Merge branch 'master' into MM-133
ayusht2810 Feb 1, 2024
6c05f40
[MM-133] Update system console and some review fixes
ayusht2810 Feb 2, 2024
417a123
[MM-133] Add some test cases
ayusht2810 Feb 7, 2024
fe29a06
[MM-133] Updated a test case
ayusht2810 Feb 7, 2024
645f26f
Merge branch 'master' of github.com:mattermost/mattermost-plugin-zoom…
ayusht2810 Feb 27, 2024
2549a1e
[MM-133] Remove unused variable
ayusht2810 Feb 27, 2024
eb5981d
Merge branch 'master' of github.com:mattermost/mattermost-plugin-zoom…
ayusht2810 Apr 25, 2024
4c013a8
[MM-133]: review fixes
Kshitij-Katiyar Jun 6, 2024
ebc3bca
[MM-133]: fixed server testcases
Kshitij-Katiyar Jun 6, 2024
66fc89c
[MM-133]: updated the channel setting list values
Kshitij-Katiyar Jun 11, 2024
5ff77f7
[MM-133]: resolved conflict
Kshitij-Katiyar Jun 21, 2024
ad2e197
Merge branch 'master' of github.com:mattermost/mattermost-plugin-zoom…
raghavaggarwal2308 Jul 2, 2024
df05023
[MM-542] Review fixes:
raghavaggarwal2308 Jul 2, 2024
8583cac
[MM-542] Fixed confusion in channel settings modal
raghavaggarwal2308 Jul 2, 2024
5f0a6bf
[MM-546] Remove bin folder
raghavaggarwal2308 Jul 4, 2024
6eb844d
[MM-133] Fix issue of list empty incase of no setting is present
ayusht2810 Jul 15, 2024
8304f0a
Merge branch 'master' of github.com:mattermost/mattermost-plugin-zoom…
ayusht2810 Jul 15, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion server/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -502,15 +502,15 @@

restrict, statusCode, err := p.checkChannelPreference(req.ChannelID)
if err != nil {
p.API.LogError("Unable to check channel preference", "ChannelID", req.ChannelID, "Error", err.Error())
http.Error(w, err.Error(), statusCode)
return
}

Check warning on line 508 in server/http.go

View check run for this annotation

Codecov / codecov/patch

server/http.go#L505-L508

Added lines #L505 - L508 were not covered by tests

if restrict {
if _, err = w.Write([]byte(`{"error": "Creating zoom meeting is disabled for this channel."}`)); err != nil {
mickmister marked this conversation as resolved.
Show resolved Hide resolved
p.API.LogWarn("failed to write the response", "error", err.Error())
}

Check warning on line 513 in server/http.go

View check run for this annotation

Codecov / codecov/patch

server/http.go#L512-L513

Added lines #L512 - L513 were not covered by tests
return
}

Expand All @@ -527,7 +527,7 @@

zoomUser, authErr := p.authenticateAndFetchZoomUser(user)
if authErr != nil {
if _, err = w.Write([]byte(`{"meeting_url": ""}`)); err != nil {

Check warning on line 530 in server/http.go

View check run for this annotation

Codecov / codecov/patch

server/http.go#L530

Added line #L530 was not covered by tests
mickmister marked this conversation as resolved.
Show resolved Hide resolved
p.API.LogWarn("failed to write the response", "error", err.Error())
}

Expand All @@ -548,7 +548,7 @@
}

if recentMeeting {
if _, err = w.Write([]byte(`{"meeting_url": ""}`)); err != nil {

Check warning on line 551 in server/http.go

View check run for this annotation

Codecov / codecov/patch

server/http.go#L551

Added line #L551 was not covered by tests
p.API.LogWarn("failed to write the response", "error", err.Error())
}
p.postConfirm(recentMeetingLink, req.ChannelID, req.Topic, userID, req.RootID, creatorName, provider)
Expand Down Expand Up @@ -940,7 +940,7 @@
func (p *Plugin) checkChannelPreference(channelID string) (bool, int, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
func (p *Plugin) checkChannelPreference(channelID string) (bool, int, error) {
func (p *Plugin) isChannelRestrictedForMeetings(channelID string) (bool, int, error) {

Not sure why we need the http status code to be returned from here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Returning statusCode for any error that arises so we can return the respective error.

Copy link
Member

Choose a reason for hiding this comment

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

Right but the only values returned are http.StatusInternalServerError and http.StatusOK, which can instead be inferred by the caller if an error happened or not

channel, appErr := p.API.GetChannel(channelID)
if appErr != nil {
return false, http.StatusInternalServerError, appErr
return false, http.StatusInternalServerError, errors.New(appErr.Message)
ayusht2810 marked this conversation as resolved.
Show resolved Hide resolved
}

zoomChannelSettingsMap, err := p.listZoomChannelSettings()
Expand All @@ -955,11 +955,11 @@
Check if creating meeting is disabled in the plugin configuration.
*/
if exist {
if val.Preference == ZoomChannelPreferences[DefaultPreference] {
mickmister marked this conversation as resolved.
Show resolved Hide resolved
preference = p.configuration.RestrictMeetingCreation
} else if val.Preference == ZoomChannelPreferences[EnablePreference] {
preference = true
}

Check warning on line 962 in server/http.go

View check run for this annotation

Codecov / codecov/patch

server/http.go#L958-L962

Added lines #L958 - L962 were not covered by tests
} else if channel.Type == model.ChannelTypeOpen {
preference = p.configuration.RestrictMeetingCreation
}
Expand All @@ -973,8 +973,8 @@
}

if mv.Preference == "" {
return errors.New("preference should not be empty")
}

Check warning on line 977 in server/http.go

View check run for this annotation

Codecov / codecov/patch

server/http.go#L976-L977

Added lines #L976 - L977 were not covered by tests

return nil
}
259 changes: 244 additions & 15 deletions server/plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package main
import (
"encoding/json"
"fmt"
"io"
"net/http"
"net/http/httptest"
"path/filepath"
Expand Down Expand Up @@ -58,6 +59,9 @@ func TestPlugin(t *testing.T) {

noAuthMeetingRequest := httptest.NewRequest("POST", "/api/v1/meetings", strings.NewReader("{\"channel_id\": \"thechannelid\"}"))

restrictMeetingRequest := httptest.NewRequest("POST", "/api/v1/meetings", strings.NewReader("{\"channel_id\": \"thechannelid\"}"))
restrictMeetingRequest.Header.Add("Mattermost-User-Id", "theuserid")

meetingRequest := httptest.NewRequest("POST", "/api/v1/meetings", strings.NewReader("{\"channel_id\": \"thechannelid\"}"))
meetingRequest.Header.Add("Mattermost-User-Id", "theuserid")

Expand All @@ -71,28 +75,27 @@ func TestPlugin(t *testing.T) {
unauthorizedUserRequest := httptest.NewRequest("POST", "/api/v1/meetings", strings.NewReader("{\"channel_id\": \"thechannelid\", \"personal\": true}"))
unauthorizedUserRequest.Header.Add("Mattermost-User-Id", "theuserid")

channelSettingsMap := ZoomChannelSettingsMap{
"thechannelid": ZoomChannelSettingsMapValue{
ChannelName: "mockChannel",
Preference: ZoomChannelPreferences[DisablePreference],
},
}

channelSettingsMapByte, _ := json.Marshal(channelSettingsMap)

for name, tc := range map[string]struct {
Request *http.Request
ExpectedStatusCode int
MeetingAllowed bool
HasPermissionToChannel bool
}{
"UnauthorizedMeetingRequest": {
Request: noAuthMeetingRequest,
ExpectedStatusCode: http.StatusUnauthorized,
HasPermissionToChannel: true,
},
"RestrictMeetingRequest": {
Request: restrictMeetingRequest,
ExpectedStatusCode: http.StatusOK,
MeetingAllowed: false,
HasPermissionToChannel: true,
},
"ValidMeetingRequest": {
Request: meetingRequest,
ExpectedStatusCode: http.StatusOK,
MeetingAllowed: true,
HasPermissionToChannel: true,
},
"ValidStoppedWebhookRequest": {
Expand Down Expand Up @@ -159,7 +162,7 @@ func TestPlugin(t *testing.T) {

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("KVGet", zoomChannelSettings).Return(channelSettingsMapByte, nil)
api.On("KVGet", zoomChannelSettings).Return([]byte{}, nil)

api.On("KVDelete", fmt.Sprintf("%v%v", postMeetingKey, 234)).Return(nil)

Expand Down Expand Up @@ -189,11 +192,12 @@ func TestPlugin(t *testing.T) {

p := Plugin{}
p.setConfiguration(&configuration{
ZoomAPIURL: ts.URL,
WebhookSecret: "thewebhooksecret",
EncryptionKey: "4Su-mLR7N6VwC6aXjYhQoT0shtS9fKz+",
OAuthClientID: "clientid",
OAuthClientSecret: "clientsecret",
ZoomAPIURL: ts.URL,
WebhookSecret: "thewebhooksecret",
EncryptionKey: "4Su-mLR7N6VwC6aXjYhQoT0shtS9fKz+",
OAuthClientID: "clientid",
OAuthClientSecret: "clientsecret",
RestrictMeetingCreation: true,
})
p.SetAPI(api)
p.tracker = telemetry.NewTracker(nil, "", "", "", "", "", false)
Expand All @@ -209,3 +213,228 @@ func TestPlugin(t *testing.T) {
})
}
}

func TestHandleChannelPreference(t *testing.T) {
for _, test := range []struct {
Name string
SetupAPI func(*plugintest.API)
RequestBody string
ExpectedStatusCode int
ExpectedResult string
}{
{
Name: "HandleChannelPreference: invalid body",
SetupAPI: func(api *plugintest.API) {
api.On("LogError", "Error decoding dialog request", "Error", mock.AnythingOfType("string")).Once()
},
RequestBody: `{
"user_id":
}`,
ExpectedStatusCode: http.StatusBadRequest,
ExpectedResult: "invalid character '}' looking for beginning of value\n",
},
{
Name: "HandleChannelPreference: empty user ID",
SetupAPI: func(api *plugintest.API) {
api.On("LogError", "Invalid user ID", "UserID", "").Once()
},
RequestBody: `{
"user_id": ""
}`,
ExpectedStatusCode: http.StatusUnauthorized,
ExpectedResult: "Not authorized\n",
},
{
Name: "HandleChannelPreference: insufficient permissions",
SetupAPI: func(api *plugintest.API) {
api.On("LogError", "Unable to resolve request due to insufficient permissions", "UserID", "mockUserID").Once()
api.On("HasPermissionTo", "mockUserID", model.PermissionManageSystem).Return(false).Once()
},
RequestBody: `{
"user_id": "mockUserID"
}`,
ExpectedStatusCode: http.StatusForbidden,
ExpectedResult: "Insufficient permissions\n",
},
{
Name: "HandleChannelPreference: invalid reqest body",
SetupAPI: func(api *plugintest.API) {
api.On("LogError", "Invalid request body", "Error", "channel name should not be empty").Once()
api.On("HasPermissionTo", "mockUserID", model.PermissionManageSystem).Return(true).Once()
},
RequestBody: `{
"user_id": "mockUserID",
"callback_id": "",
"channel_id": "mockChannelID",
"submission": {
"preference": "Enabled"
}
}`,
ExpectedStatusCode: http.StatusBadRequest,
ExpectedResult: "channel name should not be empty\n",
},
{
Name: "HandleChannelPreference: unable to set preference",
SetupAPI: func(api *plugintest.API) {
api.On("LogError", "Error setting channel preference", "Error", "unable to set preference").Once()
api.On("HasPermissionTo", "mockUserID", model.PermissionManageSystem).Return(true).Once()
api.On("KVGet", zoomChannelSettings).Return([]byte{}, nil).Once()
api.On("KVSet", zoomChannelSettings, mock.Anything).Return(&model.AppError{
Message: "unable to set preference",
}).Once()
},
RequestBody: `{
"user_id": "mockUserID",
"callback_id": "mockCallbackID",
"channel_id": "mockChannelID",
"submission": {
"preference": "Enabled"
}
}`,
ExpectedStatusCode: http.StatusInternalServerError,
ExpectedResult: "unable to set preference\n",
},
{
Name: "HandleChannelPreference: success",
SetupAPI: func(api *plugintest.API) {
api.On("HasPermissionTo", "mockUserID", model.PermissionManageSystem).Return(true).Once()
api.On("KVGet", zoomChannelSettings).Return([]byte{}, nil).Once()
api.On("KVSet", zoomChannelSettings, mock.Anything).Return(nil).Once()
},
RequestBody: `{
"user_id": "mockUserID",
"callback_id": "mockCallbackID",
"channel_id": "mockChannelID",
"submission": {
"preference": "Enabled"
}
}`,
ExpectedStatusCode: http.StatusOK,
},
} {
t.Run(test.Name, func(t *testing.T) {
assert := assert.New(t)

api := &plugintest.API{}
p := Plugin{}
p.SetAPI(api)
p.setConfiguration(&configuration{
ZoomAPIURL: "mockURL",
WebhookSecret: "thewebhooksecret",
EncryptionKey: "4Su-mLR7N6VwC6aXjYhQoT0shtS9fKz+",
OAuthClientID: "clientid",
OAuthClientSecret: "clientsecret",
})

test.SetupAPI(api)

licenseTrue := true
api.On("GetLicense").Return(&model.License{
Features: &model.Features{
Cloud: &licenseTrue,
},
})

defer api.AssertExpectations(t)

w := httptest.NewRecorder()
r := httptest.NewRequest(http.MethodPost, pathChannelPreference, strings.NewReader(test.RequestBody))
r.Header.Add("Mattermost-User-Id", "mockUserID")
p.ServeHTTP(&plugin.Context{}, w, r)

result := w.Result()
require.NotNil(t, result)
defer result.Body.Close()

bodyBytes, err := io.ReadAll(result.Body)
assert.Nil(err)

bodyString := string(bodyBytes)
assert.Equal(bodyString, test.ExpectedResult)
assert.Equal(result.StatusCode, test.ExpectedStatusCode)
})
}
}

func TestCheckChannelPreference(t *testing.T) {
for _, test := range []struct {
Name string
SetupAPI func(*plugintest.API)
ExpectedPreference bool
ExpectedStatusCode int
ExpectedError string
}{
{
Name: "CheckChannelPreference: unable to get channel",
SetupAPI: func(api *plugintest.API) {
api.On("GetChannel", "mockChannelID").Return(nil, &model.AppError{
Message: "unable to get channel",
}).Once()
},
ExpectedPreference: false,
ExpectedStatusCode: http.StatusInternalServerError,
ExpectedError: "unable to get channel",
},
{
Name: "CheckChannelPreference: unable to get preference",
SetupAPI: func(api *plugintest.API) {
api.On("GetChannel", "mockChannelID").Return(&model.Channel{
Id: "mockChannelID",
Type: model.ChannelTypeOpen,
}, nil).Once()
api.On("KVGet", zoomChannelSettings).Return(nil, &model.AppError{
Message: "unable to get preference",
}).Once()
},
ExpectedPreference: false,
ExpectedStatusCode: http.StatusInternalServerError,
ExpectedError: "unable to get preference",
},
{
Name: "CheckChannelPreference: preference not set and channel is public",
SetupAPI: func(api *plugintest.API) {
api.On("GetChannel", "mockChannelID").Return(&model.Channel{
Id: "mockChannelID",
Type: model.ChannelTypeOpen,
}, nil).Once()
api.On("KVGet", zoomChannelSettings).Return([]byte{}, nil).Once()
},
ExpectedPreference: true,
ExpectedStatusCode: http.StatusOK,
},
{
Name: "CheckChannelPreference: preference not set and channel is private",
SetupAPI: func(api *plugintest.API) {
api.On("GetChannel", "mockChannelID").Return(&model.Channel{
Id: "mockChannelID",
Type: model.ChannelTypePrivate,
}, nil).Once()
api.On("KVGet", zoomChannelSettings).Return([]byte{}, nil).Once()
},
ExpectedPreference: false,
ExpectedStatusCode: http.StatusOK,
},
} {
t.Run(test.Name, func(t *testing.T) {
assert := assert.New(t)

api := &plugintest.API{}
p := Plugin{}
p.SetAPI(api)
p.setConfiguration(&configuration{
RestrictMeetingCreation: true,
})

test.SetupAPI(api)

preference, statusCode, err := p.checkChannelPreference("mockChannelID")
assert.Equal(test.ExpectedPreference, preference)
assert.Equal(test.ExpectedStatusCode, statusCode)
if err != nil {
assert.Equal(test.ExpectedError, err.Error())
} else {
assert.Nil(err)
}
})
}
}
6 changes: 3 additions & 3 deletions server/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,14 +218,14 @@
func (p *Plugin) storeZoomChannelSettings(channelID string, zoomChannelSettingsMapValue ZoomChannelSettingsMapValue) error {
bytes, appErr := p.API.KVGet(zoomChannelSettings)
mickmister marked this conversation as resolved.
Show resolved Hide resolved
if appErr != nil {
return appErr
return errors.New(appErr.Message)
}

Check warning on line 222 in server/store.go

View check run for this annotation

Codecov / codecov/patch

server/store.go#L221-L222

Added lines #L221 - L222 were not covered by tests

var zoomChannelSettingsMap ZoomChannelSettingsMap
if len(bytes) != 0 {
if err := json.Unmarshal(bytes, &zoomChannelSettingsMap); err != nil {
return err
}

Check warning on line 228 in server/store.go

View check run for this annotation

Codecov / codecov/patch

server/store.go#L226-L228

Added lines #L226 - L228 were not covered by tests
} else {
zoomChannelSettingsMap = ZoomChannelSettingsMap{}
}
Expand All @@ -233,11 +233,11 @@
zoomChannelSettingsMap[channelID] = zoomChannelSettingsMapValue
bytes, err := json.Marshal(zoomChannelSettingsMap)
if err != nil {
return err
}

Check warning on line 237 in server/store.go

View check run for this annotation

Codecov / codecov/patch

server/store.go#L236-L237

Added lines #L236 - L237 were not covered by tests

if appErr := p.API.KVSet(zoomChannelSettings, bytes); appErr != nil {
return appErr
return errors.New(appErr.Message)
}

return nil
Expand All @@ -246,17 +246,17 @@
func (p *Plugin) listZoomChannelSettings() (ZoomChannelSettingsMap, error) {
bytes, appErr := p.API.KVGet(zoomChannelSettings)
if appErr != nil {
return nil, appErr
return nil, errors.New(appErr.Message)
}

if len(bytes) == 0 {
return ZoomChannelSettingsMap{}, nil
}

var zoomChannelSettingsMap ZoomChannelSettingsMap
if err := json.Unmarshal(bytes, &zoomChannelSettingsMap); err != nil {
return nil, err
}

Check warning on line 259 in server/store.go

View check run for this annotation

Codecov / codecov/patch

server/store.go#L256-L259

Added lines #L256 - L259 were not covered by tests

return zoomChannelSettingsMap, nil

Check warning on line 261 in server/store.go

View check run for this annotation

Codecov / codecov/patch

server/store.go#L261

Added line #L261 was not covered by tests
}
Loading