[GH-326] Added subscription flag for filtering out confidential issues, and disallow guests from creating subscriptions by Kshitij-Katiyar · Pull Request #376 · mattermost/mattermost-plugin-gitlab

Kshitij-Katiyar · 2023-05-04T10:11:35Z

Summary

Added an additional flag for confidential issues in the subscription slash command.
Added the check that guest users cannot create a subscription.

Issue

Issue #326

… subscriptions (#37) * [MI-3045]:Added checks for confidential issues * [MI-3045]:Added unit test cases * [MI-3045]:Fixed self review comments * [MI-3045]:Fixed unit test cases * [MI-3045]:Fixed lint errors * [MI-3045]:Fixed review comments * [MI-3045]:Fixed context deadline error * [MI-3045]:Fixed review comments * [MI-3045]:Fixed statements

mattermost-build · 2023-05-04T10:11:45Z

Hello @Kshitij-Katiyar,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

codecov · 2023-05-04T10:17:39Z

Codecov Report

Attention: Patch coverage is 75.00000% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 33.99%. Comparing base (98fdf4f) to head (bfb8f9c).
Report is 11 commits behind head on master.

Files	Patch %	Lines
server/subscription/subscription.go	0.00%	2 Missing ⚠️
server/webhook/issue.go	60.00%	1 Missing and 1 partial ⚠️
server/command.go	83.33%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #376      +/-   ##
==========================================
+ Coverage   33.40%   33.99%   +0.59%     
==========================================
  Files          22       22              
  Lines        3979     3992      +13     
==========================================
+ Hits         1329     1357      +28     
+ Misses       2519     2499      -20     
- Partials      131      136       +5

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

server/webhook.go

server/command.go

mickmister · 2023-05-04T14:48:24Z

server/command.go

 * |/gitlab subscriptions add owner[/repo] [features]| - Subscribe the current channel to receive notifications about opened merge requests and issues for a group or repository
  * |features| is a comma-delimited list of one or more the following:
    * issues - includes new and closed issues
+	* confidential_issues - includes new and closed confidential issues


We may want to make this opt-in, so as to not break existing subscriptions. If this is the case, the flag should be exclude_confidential_issues. Thoughts on this? @jupenur @esarafianou

@mickmister Fixed all the review comments apart for this one. Waiting for replies.

@hanzei Do you have an opinion on this?

@mickmister Hadn't been notified about this ping and bumped into it today.

The ideal behavior is secure by default. That would mean excluding confidential issues by default.

Since this is a breaking change for existing subscriptions, we can consider a major release.

server/command_test.go

…#38) * [MI-3060]:Fixed review comments of PR mattermost#376 on gitlab plugin * [MI-3060]:Fixed test cases * [MI-3060]:Updated the msg * [MI-3060]:Fixed the test cases

mattermost-build · 2023-05-19T01:01:57Z

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

mickmister · 2023-05-19T14:07:42Z

@Kshitij-Katiyar Heads up that there are conflicts to resolve here

Kshitij-Katiyar · 2023-05-25T11:50:12Z

@mickmister Fixed the merge conflicts

server/webhook.go

mickmister · 2023-05-25T13:35:41Z

server/command.go

 * |/gitlab subscriptions add owner[/repo] [features]| - Subscribe the current channel to receive notifications about opened merge requests and issues for a group or repository
  * |features| is a comma-delimited list of one or more the following:
    * issues - includes new and closed issues
+	* confidential_issues - includes new and closed confidential issues


@hanzei Do you have an opinion on this?

spirosoik

LGTM

mattermost-build · 2023-06-09T01:03:01Z

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

hanzei

LGTM

server/command.go

server/webhook.go

hanzei · 2023-06-23T14:50:47Z

@DHaussermann The PR is ready for your review

mattermost-build · 2023-07-07T01:03:17Z

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

mickmister · 2023-09-07T15:15:31Z

@Kshitij-Katiyar Heads up that there are conflicts to resolve here

hanzei · 2023-11-29T12:23:00Z

@Kshitij-Katiyar gentle reminder to resolve the conflicts

…ab into MM-326

ayusht2810 · 2024-01-03T14:44:36Z

@hanzei resolved the conflicts.

AayushChaudhary0001 · 2024-02-21T11:42:57Z

@raghavaggarwal2308 While testing this PR, none of the above conditions were found working. The guest user on MM was able to create a subscription and also there wasn't any flag for confidential issues in subscription slash command. Please go-through this once.

raghavaggarwal2308 · 2024-02-26T11:25:34Z

@raghavaggarwal2308 While testing this PR, none of the above conditions were found working. The guest user on MM was able to create a subscription and also there wasn't any flag for confidential issues in subscription slash command. Please go-through this once.

@AayushChaudhary0001

The flag is added in the "--features" section while creating a subscription.
The user who has guest access to a Gitlab project cannot create a subscription on MM.

AayushChaudhary0001

Thanks @raghavaggarwal2308 for the assistance.

The above PR has been tested for the following features:-

There is a seperate flag added for creating a subscription for confidential issues.
A user with a guest access of any repo on Gitlab cannot create a subscription in MM.

The PR was working fine for both the above conditions, LGTM. Approved.

Kshitij-Katiyar requested review from mickmister and spirosoik as code owners May 4, 2023 10:11

mickmister reviewed May 4, 2023

View reviewed changes

mickmister changed the title ~~[GH-326] Added checks for confidential issues at the time of making subscriptions~~ [GH-326] Added subscription flag for filtering out confidential issues, and disallow guests from creating subscriptions May 4, 2023

Kshitij-Katiyar added a commit to Brightscout/mattermost-plugin-gitlab that referenced this pull request May 5, 2023

[MI-3060]:Fixed review comments of PR mattermost#376 on gitlab plugin

eb231e2

Kshitij-Katiyar mentioned this pull request May 5, 2023

[MI-3060]:Fixed review comments of PR #376 on gitlab plugin Brightscout/mattermost-plugin-gitlab#38

Merged

[MI-3060]:Fixed review comments of PR mattermost#376 on gitlab plugin (…

ddbc11f

…#38) * [MI-3060]:Fixed review comments of PR mattermost#376 on gitlab plugin * [MI-3060]:Fixed test cases * [MI-3060]:Updated the msg * [MI-3060]:Fixed the test cases

Kshitij-Katiyar requested a review from mickmister May 8, 2023 08:02

mickmister approved these changes May 8, 2023

View reviewed changes

mattermost-build added the Lifecycle/1:stale label May 19, 2023

mickmister added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester and removed Lifecycle/1:stale labels May 19, 2023

mickmister requested a review from DHaussermann May 19, 2023 14:07

hanzei added the Awaiting Submitter Action Blocked on the author label May 21, 2023

Kshitij-Katiyar added 3 commits May 25, 2023 17:03

[MM-326]:Fix conflicts

b935acf

[MM-326]:fixed test cases

5eeb262

[MM-326]:Fixed CI error

0e6aa09

mickmister reviewed May 25, 2023

View reviewed changes

[MM-326]:Fixed review comments

2daa89e

spirosoik approved these changes May 29, 2023

View reviewed changes

mattermost-build added the Lifecycle/1:stale label Jun 9, 2023

hanzei self-requested a review June 9, 2023 07:41

hanzei approved these changes Jun 23, 2023

View reviewed changes

server/command.go Outdated Show resolved Hide resolved

server/webhook.go Outdated Show resolved Hide resolved

hanzei removed 2: Dev Review Requires review by a core committer Awaiting Submitter Action Blocked on the author Lifecycle/1:stale labels Jun 23, 2023

[MM-326]:Fixed review comments

017d014

mattermost-build added the Lifecycle/1:stale label Jul 7, 2023

mickmister approved these changes Jul 11, 2023

View reviewed changes

hanzei added the Awaiting Submitter Action Blocked on the author label Sep 7, 2023

hanzei removed the request for review from DHaussermann October 16, 2023 20:43

avas27JTG added this to the v1.9.0 milestone Dec 5, 2023

Merge branch 'master' of github.com:mattermost/mattermost-plugin-gitl…

bfb8f9c

…ab into MM-326

hanzei removed Awaiting Submitter Action Blocked on the author Lifecycle/1:stale labels Jan 4, 2024

hanzei requested a review from DHaussermann January 4, 2024 12:02

hanzei requested review from AayushChaudhary0001 and removed request for DHaussermann January 23, 2024 14:28

AayushChaudhary0001 approved these changes Feb 28, 2024

View reviewed changes

hanzei added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester labels Feb 28, 2024

avas27JTG merged commit 54a9c46 into mattermost:master Mar 8, 2024

avas27JTG deleted the MM-326 branch March 8, 2024 13:34

ayusht2810 mentioned this pull request Mar 22, 2024

Release 1.9.0 #448

Closed

raghavaggarwal2308 mentioned this pull request Jul 1, 2024

why do GitLab Guest see confidential issues ? #326

Closed

Conversation

Kshitij-Katiyar commented May 4, 2023

Uh oh!

mattermost-build commented May 4, 2023

Uh oh!

codecov bot commented May 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mickmister May 4, 2023

Choose a reason for hiding this comment

Uh oh!

Kshitij-Katiyar May 8, 2023

Choose a reason for hiding this comment

Uh oh!

mickmister May 25, 2023

Choose a reason for hiding this comment

Uh oh!

esarafianou May 29, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mattermost-build commented May 19, 2023

Uh oh!

mickmister commented May 19, 2023

Uh oh!

Kshitij-Katiyar commented May 25, 2023

Uh oh!

Uh oh!

mickmister May 25, 2023

Choose a reason for hiding this comment

Uh oh!

spirosoik left a comment

Choose a reason for hiding this comment

Uh oh!

mattermost-build commented Jun 9, 2023

Uh oh!

hanzei left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

hanzei commented Jun 23, 2023

Uh oh!

mattermost-build commented Jul 7, 2023

Uh oh!

mickmister commented Sep 7, 2023

Uh oh!

hanzei commented Nov 29, 2023

Uh oh!

ayusht2810 commented Jan 3, 2024

Uh oh!

AayushChaudhary0001 commented Feb 21, 2024

Uh oh!

raghavaggarwal2308 commented Feb 26, 2024

Uh oh!

AayushChaudhary0001 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

codecov bot commented May 4, 2023 •

edited

Loading