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

[MM-223] Fixing Post Permissions Access #233

Merged
merged 8 commits into from
Nov 11, 2021
Merged

[MM-223] Fixing Post Permissions Access #233

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5d4f4c7
[MM-223] Fixing Post Permissions Access
xantabdoc Oct 14, 2021
caace5d
Fixing CircleCI Test (#223)
xantabdoc Oct 23, 2021
3f1a743
Fixing CircleCI Test (#223)
xantabdoc Oct 25, 2021
d0f87cb
Merge branch 'master' into MM-223
mattermod Oct 26, 2021
1cc56e8
Merge branch 'master' into MM-223
hanzei Oct 28, 2021
88ad0af
Fixing Post Permissions Access (#223)
Nov 4, 2021
ae36c46
Fixing Post Permissions Access (#223)
Nov 4, 2021
3b99b92
Fixing Post Permissions Access (#223)
Nov 4, 2021
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
4 changes: 4 additions & 0 deletions server/http.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,10 @@ func (p *Plugin) postMeeting(creator *model.User, meetingID int, channelID strin
topic = defaultMeetingTopic
}

if !p.API.HasPermissionToChannel(creator.Id, channelID, model.PERMISSION_CREATE_POST) {
return errors.New("this channel is not accessible, you might not have permissions to write in this channel. Contact the administrator of this channel to find out if you have access permissions")
Copy link
Contributor

Choose a reason for hiding this comment

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

@vicky-demansol We still need a unit test that covers the inside of this if statement

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was add the unit test but the server response will return http code = 500, this response could be it returned as "forbidden" or "unauthorized"

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @vicky-demansol, please make sure you mention the person you're replying to on GitHub issues/PRs, so we can stay up-to-date on discussions like this.

I was add the unit test but the server response will return http code = 500, this response could be it returned as "forbidden" or "unauthorized"

Can we create a unit test just to cover that this line will run? If it take a lot of restructure to accomplish changing the response code, let's just make sure this error occurs in general.

Copy link
Contributor Author

@jupriano jupriano Nov 9, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@mickmister i was trying, if we want to get http code error need to use appError but i don't think this function can use in general, but this code on code coverage was covered

}

slackAttachment := model.SlackAttachment{
Fallback: fmt.Sprintf("Video Meeting started at [%d](%s).\n\n[Join Meeting](%s)", meetingID, meetingURL, meetingURL),
Title: topic,
Expand Down
40 changes: 28 additions & 12 deletions server/plugin_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,29 +53,43 @@ func TestPlugin(t *testing.T) {

noSecretWebhookRequest := httptest.NewRequest("POST", "/webhook", strings.NewReader(endedPayload))

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

for name, tc := range map[string]struct {
Request *http.Request
ExpectedStatusCode int
Request *http.Request
ExpectedStatusCode int
HasPermissionToChannel bool
}{
"UnauthorizedMeetingRequest": {
Request: noAuthMeetingRequest,
ExpectedStatusCode: http.StatusUnauthorized,
Request: noAuthMeetingRequest,
ExpectedStatusCode: http.StatusUnauthorized,
HasPermissionToChannel: true,
},
"ValidPersonalMeetingRequest": {
Request: personalMeetingRequest,
ExpectedStatusCode: http.StatusOK,
Request: personalMeetingRequest,
ExpectedStatusCode: http.StatusOK,
HasPermissionToChannel: true,
},
"ValidStoppedWebhookRequest": {
Request: validStoppedWebhookRequest,
ExpectedStatusCode: http.StatusOK,
Request: validStoppedWebhookRequest,
ExpectedStatusCode: http.StatusOK,
HasPermissionToChannel: true,
},
"ValidStartedWebhookRequest": {
Request: validStartedWebhookRequest,
ExpectedStatusCode: http.StatusNotImplemented,
Request: validStartedWebhookRequest,
ExpectedStatusCode: http.StatusNotImplemented,
HasPermissionToChannel: true,
},
"NoSecretWebhookRequest": {
Request: noSecretWebhookRequest,
ExpectedStatusCode: http.StatusUnauthorized,
Request: noSecretWebhookRequest,
ExpectedStatusCode: http.StatusUnauthorized,
HasPermissionToChannel: true,
},
"UnauthorizedChannelPermissions": {
Request: unauthorizedUserRequest,
ExpectedStatusCode: http.StatusInternalServerError,
HasPermissionToChannel: false,
},
} {
t.Run(name, func(t *testing.T) {
Expand All @@ -88,6 +102,8 @@ func TestPlugin(t *testing.T) {
Email: "theuseremail",
}, nil)

api.On("HasPermissionToChannel", "theuserid", "thechannelid", model.PERMISSION_CREATE_POST).Return(tc.HasPermissionToChannel)

api.On("GetChannelMember", "thechannelid", "theuserid").Return(&model.ChannelMember{}, nil)

api.On("GetPost", "thepostid").Return(&model.Post{Props: map[string]interface{}{}}, nil)
Expand Down