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

Support filtering on comment visibility for subscriptions #626

Open
mickmister opened this issue Aug 17, 2020 · 9 comments · May be fixed by #894
Open

Support filtering on comment visibility for subscriptions #626

mickmister opened this issue Aug 17, 2020 · 9 comments · May be fixed by #894
Labels
Difficulty/2:Medium Medium ticket Hacktoberfest Help Wanted Community help wanted Tech/Go Tech/ReactJS Tech/TypeScript Type/Enhancement New feature or improvement of existing feature
Milestone
next milestone

Comments

@mickmister
Copy link
Member

mickmister commented Aug 17, 2020 •
edited
Loading

Summary

When creating a comment in Jira, it's possible to make it so the comment is only visible to specific groups of users. This is a security issue because a private comment could be posted to a channel that has MM users that should not see this comment. The subscriptions could support filtering on certain Jira groups.

When a comment is created/updated, the Jira plugin is notified through a webhook. The comment payload does not contain information on the comment visibility, so we need to fetch the comment during this time of processing the webhook call. This should only be done if necessary (if the subscription has a filter set up to filter on comment visibility). In order to ensure we get all possible information about the comment when we fetch it, we should use the author's access token to fetch the comment. We can use the URL in the webhook payload located at Comment.Self:

mattermost-plugin-jira/server/testdata/webhook-issue-comment-created-indentation.json

Lines 122 to 125 in 7c1ed9b

"comments": [
{
"self": "http://localhost:8082/rest/api/2/issue/10101/comment/10503",
"id": "10503",

The subscription modal should support a new "field" to filter on, Comment Visibility. We will need to have an option to allow/reject certain visibilities, just as the other fields work.

We can fetch the available roles using Jira's project API rest/api/2/project/{project_key}, then examine the Roles property of the project.
@mickmister mickmister added Help Wanted Community help wanted Up For Grabs Ready for help from the community. Removed when someone volunteers Tech/ReactJS Tech/Go Difficulty/2:Medium Medium ticket Tech/TypeScript Type/Enhancement New feature or improvement of existing feature labels Aug 17, 2020
@mickmister mickmister mentioned this issue Aug 17, 2020
Closed
@mickmister mickmister added Help Wanted Community help wanted and removed Help Wanted Community help wanted labels Nov 16, 2020
@mickmister mickmister mentioned this issue Jul 2, 2021
Open
@hanzei hanzei added this to To do in Plugin Sprint Planning via automation Jul 2, 2021
@hanzei hanzei removed the Up For Grabs Ready for help from the community. Removed when someone volunteers label Jul 2, 2021
@mickmister
Copy link
Member Author

The Comment Visibility filter should show in the dropdown for available filters at the bottom of the subscriptions modal:

image

@mickmister
Copy link
Member Author

It may be helpful to use the BackendSelector component introduced in #583, to reuse code being used to introduce a new filter selector.

@deepakdemiwal deepakdemiwal moved this from To do to High Priority in Plugin Sprint Planning Jul 4, 2021
@sanjaydemansol
Copy link

Hi @mickmister, please check it out

@deepakdemiwal deepakdemiwal moved this from High Priority to In Progress in Plugin Sprint Planning Jul 13, 2021
@mickmister
Copy link
Member Author

@sanjaydemansol Based on the discussion in the thread you linked, I think we are ok using the https://{org_name}.atlassian.net/rest/api/2/project/{project_key} route to fetch the comment visibility roles, even if the user is not a member of this role. This is because by default, the subscription would be allowing these comments to come through anyway. Does this make sense?

@hanzei hanzei added this to the v3.2.0 milestone Jan 4, 2022
@mickmister
Copy link
Member Author

@sanjaydemansol Please take a look at the RESTService method RESTGet:

mattermost-plugin-jira/server/client.go

Line 44 in b62a305

RESTGet(endpoint string, params map[string]string, dest interface{}) error

This is the method you can use to perform these API calls. Please take a look at pieces of code that are using this method

@hanzei hanzei modified the milestones: v3.2.0, v3.3.0 Mar 1, 2022
@hanzei hanzei removed this from In Progress in Plugin Sprint Planning Mar 16, 2022
@Nityanand13
Copy link
Contributor

Nityanand13 commented Nov 10, 2022 •
edited
Loading

@mickmister Please look at our plan for solving this issue.
JIRA Issue 626 PLAN.pdf

@Nityanand13
Copy link
Contributor

Nityanand13 commented Nov 11, 2022 •
edited
Loading

@mickmister Please look at our plan for solving this issue. JIRA Issue 626 PLAN.pdf

@mickmister I have updated this comment with the new pdf

@mickmister
Copy link
Member Author

mickmister commented Nov 17, 2022 •
edited
Loading

@Nityanand13 The document looks good to me 👍

For the last section:

But there is a bug in the above-proposed solution:-
If a user who has created a subscription is removed from the particular group which was added in the filter of Jira subscription. And now, another user creates a comment with the same visibility attribute then in that case a user who has created a subscription does not have access to that comment. So we will not get the notification of that comment but technically we should get it.

Just to be clear, this situation is:

  • Commenter has not connected their account, and
  • Subscription creator is not a member of group, or is no longer connected

I think the behavior for this is fine (not creating the post). I think logging a message with log level INFO is all that needs to be done.

@Nityanand13 Nityanand13 linked a pull request Nov 28, 2022 that will close this issue
Open
@wiersgallak
Copy link
Contributor

Closing issues due to inactivity. This issue can be re-opened with more interest from our community.

@wiersgallak wiersgallak reopened this Jun 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Difficulty/2:Medium Medium ticket Hacktoberfest Help Wanted Community help wanted Tech/Go Tech/ReactJS Tech/TypeScript Type/Enhancement New feature or improvement of existing feature
Projects
Brightscout Plugin Maintenance
Status: Submitted / In Review
Milestone
next milestone
7 participants
@jespino @cpanato @mickmister @hanzei @wiersgallak @sanjaydemansol @Nityanand13