From dffc4927e8757fd7498b319d49b6b6a7b0590d32 Mon Sep 17 00:00:00 2001 From: Michael Kochell <6913320+mickmister@users.noreply.github.com> Date: Tue, 17 May 2022 18:24:37 -0400 Subject: [PATCH 01/15] ensure user has access to selected sec level. disallow "exclude" clauses for sec level --- go.mod | 1 + server/issue.go | 13 +++++---- server/issue_test.go | 23 +++++++++++++++ server/subscribe.go | 68 +++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 98 insertions(+), 7 deletions(-) diff --git a/go.mod b/go.mod index d66e09214..9b48d7e5b 100644 --- a/go.mod +++ b/go.mod @@ -15,6 +15,7 @@ require ( github.com/pkg/errors v0.9.1 github.com/rbriski/atlassian-jwt v0.0.0-20180307182949-7bb4ae273058 github.com/stretchr/testify v1.7.0 + github.com/trivago/tgo v1.0.1 golang.org/x/oauth2 v0.0.0-20211104180415-d3ed0bb246c8 ) diff --git a/server/issue.go b/server/issue.go index c95f771f5..0a6913fe2 100644 --- a/server/issue.go +++ b/server/issue.go @@ -22,12 +22,13 @@ import ( ) const ( - labelsField = "labels" - statusField = "status" - reporterField = "reporter" - priorityField = "priority" - descriptionField = "description" - resolutionField = "resolution" + labelsField = "labels" + statusField = "status" + reporterField = "reporter" + priorityField = "priority" + descriptionField = "description" + resolutionField = "resolution" + securityLevelField = "security" ) func makePost(userID, channelID, message string) *model.Post { diff --git a/server/issue_test.go b/server/issue_test.go index 0920912cd..ddefc901c 100644 --- a/server/issue_test.go +++ b/server/issue_test.go @@ -15,6 +15,7 @@ import ( "github.com/mattermost/mattermost-server/v6/plugin/plugintest/mock" "github.com/pkg/errors" "github.com/stretchr/testify/assert" + "github.com/trivago/tgo/tcontainer" "github.com/mattermost/mattermost-plugin-jira/server/utils/kvstore" ) @@ -84,6 +85,28 @@ func (client testClient) AddComment(issueKey string, comment *jira.Comment) (*ji return nil, nil } +func (client testClient) GetCreateMeta(options *jira.GetQueryOptions) (*jira.CreateMetaInfo, error) { + return &jira.CreateMetaInfo{ + Projects: []*jira.MetaProject{ + { + IssueTypes: []*jira.MetaIssueType{ + { + Fields: tcontainer.MarshalMap{ + "security": tcontainer.MarshalMap{ + "allowedValues": []interface{}{ + tcontainer.MarshalMap{ + "id": "10001", + }, + }, + }, + }, + }, + }, + }, + }, + }, nil +} + func TestTransitionJiraIssue(t *testing.T) { api := &plugintest.API{} api.On("SendEphemeralPost", mock.Anything, mock.Anything).Return(nil) diff --git a/server/subscribe.go b/server/subscribe.go index 0792f9048..44dbd61ac 100644 --- a/server/subscribe.go +++ b/server/subscribe.go @@ -298,6 +298,36 @@ func (p *Plugin) validateSubscription(instanceID types.ID, subscription *Channel return errors.New("please provide a project identifier") } + projectKey := subscription.Filters.Projects.Elems()[0] + + var securityLevels StringSet + for _, field := range subscription.Filters.Fields { + if field.Key != securityLevelField { + continue + } + + if field.Inclusion == FilterEmpty { + continue + } + + if field.Inclusion == FilterExcludeAny { + return errors.New("security level does not allow for an \"Exclude\" clause") + } + + if securityLevels == nil { + securityLevelsArray, err := getSecurityLevelsForProject(client, projectKey) + if err != nil { + return errors.Wrap(err, "failed to get security levels for project") + } + + securityLevels = NewStringSet(securityLevelsArray...) + } + + if !securityLevels.ContainsAll(field.Values.Elems()...) { + return errors.New("invalid access to security level") + } + } + channelID := subscription.ChannelID subs, err := p.getSubscriptionsForChannel(instanceID, channelID) if err != nil { @@ -310,7 +340,6 @@ func (p *Plugin) validateSubscription(instanceID types.ID, subscription *Channel } } - projectKey := subscription.Filters.Projects.Elems()[0] _, err = client.GetProject(projectKey) if err != nil { return errors.WithMessagef(err, "failed to get project %q", projectKey) @@ -319,6 +348,43 @@ func (p *Plugin) validateSubscription(instanceID types.ID, subscription *Channel return nil } +func getSecurityLevelsForProject(client Client, projectKey string) ([]string, error) { + createMeta, err := client.GetCreateMeta(&jira.GetQueryOptions{ + Expand: "projects.issuetypes.fields", + ProjectKeys: projectKey, + }) + if err != nil { + return nil, errors.Wrap(err, "error fetching user security levels") + } + + securityLevels1, err := createMeta.Projects[0].IssueTypes[0].Fields.MarshalMap(securityLevelField) + if err != nil { + return nil, errors.Wrap(err, "error parsing user security levels") + } + + allowed, ok := securityLevels1["allowedValues"].([]interface{}) + if !ok { + return nil, errors.New("error parsing user security levels: failed to type assertion on array") + } + + out := []string{} + for _, level := range allowed { + value, ok := level.(tcontainer.MarshalMap) + if !ok { + return nil, errors.New("error parsing user security levels: failed to type assertion on map") + } + + id, ok := value["id"].(string) + if !ok { + return nil, errors.New("error parsing user security levels: failed to type assertion on string") + } + + out = append(out, id) + } + + return out, nil +} + func (p *Plugin) editChannelSubscription(instanceID types.ID, modifiedSubscription *ChannelSubscription, client Client) error { subKey := keyWithInstanceID(instanceID, JiraSubscriptionsKey) return p.atomicModify(subKey, func(initialBytes []byte) ([]byte, error) { From 1b4bc836ced73eb0a2e7ba0bfbd8c2dbb113d9bc Mon Sep 17 00:00:00 2001 From: Michael Kochell <6913320+mickmister@users.noreply.github.com> Date: Tue, 17 May 2022 18:25:08 -0400 Subject: [PATCH 02/15] if a subscription has no configured sec level, assume the issue should have a blank sec level --- server/subscribe.go | 31 ++++-- server/subscribe_test.go | 225 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 246 insertions(+), 10 deletions(-) diff --git a/server/subscribe.go b/server/subscribe.go index 44dbd61ac..5f07b0a64 100644 --- a/server/subscribe.go +++ b/server/subscribe.go @@ -15,6 +15,7 @@ import ( jira "github.com/andygrunwald/go-jira" "github.com/pkg/errors" + "github.com/trivago/tgo/tcontainer" "github.com/mattermost/mattermost-server/v6/model" @@ -141,24 +142,28 @@ func (p *Plugin) matchesSubsciptionFilters(wh *webhook, filters SubscriptionFilt return false } - if filters.IssueTypes.Len() != 0 && !filters.IssueTypes.ContainsAny(wh.JiraWebhook.Issue.Fields.Type.ID) { + issue := &wh.JiraWebhook.Issue + + if filters.IssueTypes.Len() != 0 && !filters.IssueTypes.ContainsAny(issue.Fields.Type.ID) { return false } - if filters.Projects.Len() != 0 && !filters.Projects.ContainsAny(wh.JiraWebhook.Issue.Fields.Project.Key) { + if filters.Projects.Len() != 0 && !filters.Projects.ContainsAny(issue.Fields.Project.Key) { return false } - validFilter := true - + containsSecurityLevelFilter := false for _, field := range filters.Fields { // Broken filter, values must be provided if field.Inclusion == "" || (field.Values.Len() == 0 && field.Inclusion != FilterEmpty) { - validFilter = false - break + return false + } + + if field.Key == securityLevelField { + containsSecurityLevelFilter = true } - value := getIssueFieldValue(&wh.JiraWebhook.Issue, field.Key) + value := getIssueFieldValue(issue, field.Key) containsAny := value.ContainsAny(field.Values.Elems()...) containsAll := value.ContainsAll(field.Values.Elems()...) @@ -166,12 +171,18 @@ func (p *Plugin) matchesSubsciptionFilters(wh *webhook, filters SubscriptionFilt (field.Inclusion == FilterIncludeAll && !containsAll) || (field.Inclusion == FilterExcludeAny && containsAny) || (field.Inclusion == FilterEmpty && value.Len() > 0) { - validFilter = false - break + return false + } + } + + if !containsSecurityLevelFilter { + securityLevel := getIssueFieldValue(issue, securityLevelField) + if securityLevel.Len() > 0 { + return false } } - return validFilter + return true } func (p *Plugin) getChannelsSubscribed(wh *webhook, instanceID types.ID) ([]ChannelSubscription, error) { diff --git a/server/subscribe_test.go b/server/subscribe_test.go index c9ce769a4..d3430adf9 100644 --- a/server/subscribe_test.go +++ b/server/subscribe_test.go @@ -19,6 +19,171 @@ import ( "github.com/stretchr/testify/assert" ) +func TestValidateSubscription(t *testing.T) { + p := &Plugin{} + + p.instanceStore = p.getMockInstanceStoreKV(0) + + api := &plugintest.API{} + p.SetAPI(api) + + for name, tc := range map[string]struct { + subscription *ChannelSubscription + errorMessage string + }{ + "no event selected": { + subscription: &ChannelSubscription{ + ID: "id", + Name: "name", + ChannelID: "channelid", + InstanceID: "instance_id", + Filters: SubscriptionFilters{ + Events: NewStringSet(), + Projects: NewStringSet("project"), + IssueTypes: NewStringSet("10001"), + }, + }, + errorMessage: "please provide at least one event type", + }, + "no project selected": { + subscription: &ChannelSubscription{ + ID: "id", + Name: "name", + ChannelID: "channelid", + InstanceID: "instance_id", + Filters: SubscriptionFilters{ + Events: NewStringSet("issue_created"), + Projects: NewStringSet(), + IssueTypes: NewStringSet("10001"), + }, + }, + errorMessage: "please provide a project identifier", + }, + "no issue type selected": { + subscription: &ChannelSubscription{ + ID: "id", + Name: "name", + ChannelID: "channelid", + InstanceID: "instance_id", + Filters: SubscriptionFilters{ + Events: NewStringSet("issue_created"), + Projects: NewStringSet("project"), + IssueTypes: NewStringSet(), + }, + }, + errorMessage: "please provide at least one issue type", + }, + "valid subscription": { + subscription: &ChannelSubscription{ + ID: "id", + Name: "name", + ChannelID: "channelid", + InstanceID: "instance_id", + Filters: SubscriptionFilters{ + Events: NewStringSet("issue_created"), + Projects: NewStringSet("project"), + IssueTypes: NewStringSet("10001"), + }, + }, + errorMessage: "", + }, + "valid subscription with security level": { + subscription: &ChannelSubscription{ + ID: "id", + Name: "name", + ChannelID: "channelid", + InstanceID: "instance_id", + Filters: SubscriptionFilters{ + Events: NewStringSet("issue_created"), + Projects: NewStringSet("TEST"), + IssueTypes: NewStringSet("10001"), + Fields: []FieldFilter{ + { + Key: "security", + Inclusion: FilterIncludeAll, + Values: NewStringSet("10001"), + }, + }, + }, + }, + errorMessage: "", + }, + "invalid 'Exclude' of security level": { + subscription: &ChannelSubscription{ + ID: "id", + Name: "name", + ChannelID: "channelid", + InstanceID: "instance_id", + Filters: SubscriptionFilters{ + Events: NewStringSet("issue_created"), + Projects: NewStringSet("TEST"), + IssueTypes: NewStringSet("10001"), + Fields: []FieldFilter{ + { + Key: "security", + Inclusion: FilterExcludeAny, + Values: NewStringSet("10001"), + }, + }, + }, + }, + errorMessage: "security level does not allow for an \"Exclude\" clause", + }, + "invalid access to security level": { + subscription: &ChannelSubscription{ + ID: "id", + Name: "name", + ChannelID: "channelid", + InstanceID: "instance_id", + Filters: SubscriptionFilters{ + Events: NewStringSet("issue_created"), + Projects: NewStringSet("TEST"), + IssueTypes: NewStringSet("10001"), + Fields: []FieldFilter{ + { + Key: "security", + Inclusion: FilterIncludeAll, + Values: NewStringSet("10002"), + }, + }, + }, + }, + errorMessage: "invalid access to security level", + }, + "user does not have read access to the project": { + subscription: &ChannelSubscription{ + ID: "id", + Name: "name", + ChannelID: "channelid", + InstanceID: "instance_id", + Filters: SubscriptionFilters{ + Events: NewStringSet("issue_created"), + Projects: NewStringSet(nonExistantProjectKey), + IssueTypes: NewStringSet("10001"), + }, + }, + errorMessage: "failed to get project \"FP\": Project FP not found", + }, + } { + t.Run(name, func(t *testing.T) { + api := &plugintest.API{} + p.SetAPI(api) + + api.On("KVGet", testSubKey).Return(nil, nil) + + client := testClient{} + err := p.validateSubscription(testInstance1.InstanceID, tc.subscription, client) + + if tc.errorMessage == "" { + require.NoError(t, err) + } else { + require.Error(t, err) + require.Equal(t, tc.errorMessage, err.Error()) + } + }) + } +} + func TestListChannelSubscriptions(t *testing.T) { p := &Plugin{} p.updateConfig(func(conf *config) { @@ -1357,6 +1522,66 @@ func TestGetChannelsSubscribed(t *testing.T) { }), ChannelSubscriptions: []ChannelSubscription{{ChannelID: "sampleChannelId"}}, }, + "no security level provided in subscription, but security level is present in issue": { + WebhookTestData: "webhook-issue-created-with-security-level.json", + Subs: withExistingChannelSubscriptions([]ChannelSubscription{ + { + ID: "rg86cd65efdjdjezgisgxaitzh", + ChannelID: "sampleChannelId", + Filters: SubscriptionFilters{ + Events: NewStringSet("event_created"), + Projects: NewStringSet("TES"), + IssueTypes: NewStringSet("10001"), + Fields: []FieldFilter{}, + }, + }, + }), + ChannelSubscriptions: []ChannelSubscription{}, + }, + "security level provided in subscription, but different security level is present in issue": { + WebhookTestData: "webhook-issue-created-with-security-level.json", + Subs: withExistingChannelSubscriptions([]ChannelSubscription{ + { + ID: "rg86cd65efdjdjezgisgxaitzh", + ChannelID: "sampleChannelId", + Filters: SubscriptionFilters{ + Events: NewStringSet("event_created"), + Projects: NewStringSet("TES"), + IssueTypes: NewStringSet("10001"), + Fields: []FieldFilter{ + { + Key: "security", + Inclusion: FilterIncludeAll, + Values: NewStringSet("10002"), + }, + }, + }, + }, + }), + ChannelSubscriptions: []ChannelSubscription{}, + }, + "security level provided in subscription, and same security level is present in issue": { + WebhookTestData: "webhook-issue-created-with-security-level.json", + Subs: withExistingChannelSubscriptions([]ChannelSubscription{ + { + ID: "rg86cd65efdjdjezgisgxaitzh", + ChannelID: "sampleChannelId", + Filters: SubscriptionFilters{ + Events: NewStringSet("event_created"), + Projects: NewStringSet("TES"), + IssueTypes: NewStringSet("10001"), + Fields: []FieldFilter{ + { + Key: "security", + Inclusion: FilterIncludeAll, + Values: NewStringSet("10001"), + }, + }, + }, + }, + }), + ChannelSubscriptions: []ChannelSubscription{{ChannelID: "sampleChannelId"}}, + }, } { t.Run(name, func(t *testing.T) { api := &plugintest.API{} From 329300f784dfafa77b7853040720da9020a0f580 Mon Sep 17 00:00:00 2001 From: Michael Kochell <6913320+mickmister@users.noreply.github.com> Date: Tue, 17 May 2022 18:31:35 -0400 Subject: [PATCH 03/15] add test data file --- ...ook-issue-created-with-security-level.json | 238 ++++++++++++++++++ 1 file changed, 238 insertions(+) create mode 100644 server/testdata/webhook-issue-created-with-security-level.json diff --git a/server/testdata/webhook-issue-created-with-security-level.json b/server/testdata/webhook-issue-created-with-security-level.json new file mode 100644 index 000000000..d01f91231 --- /dev/null +++ b/server/testdata/webhook-issue-created-with-security-level.json @@ -0,0 +1,238 @@ +{ + "timestamp": 1550286113023, + "webhookEvent": "jira:issue_created", + "issue_event_type_name": "issue_created", + "user": { + "self": "https://some-instance-test.atlassian.net/rest/api/2/user?accountId=5c5f880629be9642ba529340", + "name": "admin", + "key": "admin", + "accountId": "5c5f880629be9642ba529340", + "emailAddress": "some-instance-test@gmail.com", + "avatarUrls": { + "48x48": "https://avatar-cdn.atlassian.com/d991bc281c0c0ecb0bbb2db3979ddaff?s=48&d=https%3A%2F%2Fsecure.gravatar.com%2Favatar%2Fd991bc281c0c0ecb0bbb2db3979ddaff%3Fd%3Dmm%26s%3D48%26noRedirect%3Dtrue", + "24x24": "https://avatar-cdn.atlassian.com/d991bc281c0c0ecb0bbb2db3979ddaff?s=24&d=https%3A%2F%2Fsecure.gravatar.com%2Favatar%2Fd991bc281c0c0ecb0bbb2db3979ddaff%3Fd%3Dmm%26s%3D24%26noRedirect%3Dtrue", + "16x16": "https://avatar-cdn.atlassian.com/d991bc281c0c0ecb0bbb2db3979ddaff?s=16&d=https%3A%2F%2Fsecure.gravatar.com%2Favatar%2Fd991bc281c0c0ecb0bbb2db3979ddaff%3Fd%3Dmm%26s%3D16%26noRedirect%3Dtrue", + "32x32": "https://avatar-cdn.atlassian.com/d991bc281c0c0ecb0bbb2db3979ddaff?s=32&d=https%3A%2F%2Fsecure.gravatar.com%2Favatar%2Fd991bc281c0c0ecb0bbb2db3979ddaff%3Fd%3Dmm%26s%3D32%26noRedirect%3Dtrue" + }, + "displayName": "Test User", + "active": true, + "timeZone": "America/Los_Angeles" + }, + "issue": { + "id": "10040", + "self": "https://some-instance-test.atlassian.net/rest/api/2/issue/10040", + "key": "TES-41", + "fields": { + "issuetype": { + "self": "https://some-instance-test.atlassian.net/rest/api/2/issuetype/10001", + "id": "10001", + "description": "Stories track functionality or features expressed as user goals.", + "iconUrl": "https://some-instance-test.atlassian.net/secure/viewavatar?size=xsmall&avatarId=10315&avatarType=issuetype", + "name": "Story", + "subtask": false, + "avatarId": 10315 + }, + "timespent": null, + "customfield_10030": null, + "project": { + "self": "https://some-instance-test.atlassian.net/rest/api/2/project/10000", + "id": "10000", + "key": "TES", + "name": "test1", + "projectTypeKey": "software", + "avatarUrls": { + "48x48": "https://some-instance-test.atlassian.net/secure/projectavatar?avatarId=10324", + "24x24": "https://some-instance-test.atlassian.net/secure/projectavatar?size=small&avatarId=10324", + "16x16": "https://some-instance-test.atlassian.net/secure/projectavatar?size=xsmall&avatarId=10324", + "32x32": "https://some-instance-test.atlassian.net/secure/projectavatar?size=medium&avatarId=10324" + } + }, + "fixVersions": [], + "aggregatetimespent": null, + "resolution": null, + "customfield_10027": null, + "resolutiondate": null, + "workratio": -1, + "lastViewed": null, + "watches": { + "self": "https://some-instance-test.atlassian.net/rest/api/2/issue/TES-41/watchers", + "watchCount": 0, + "isWatching": true + }, + "created": "2019-02-15T19:01:52.971-0800", + "customfield_10020": null, + "customfield_10021": null, + "customfield_10022": "0|i00067:", + "priority": { + "self": "https://some-instance-test.atlassian.net/rest/api/2/priority/2", + "iconUrl": "https://some-instance-test.atlassian.net/images/icons/priorities/high.svg", + "name": "High", + "id": "2" + }, + "customfield_10023": null, + "customfield_10024": [], + "customfield_10025": null, + "customfield_10026": null, + "labels": [ + "test-label" + ], + "customfield_10016": null, + "customfield_10017": null, + "customfield_10018": { + "hasEpicLinkFieldDependency": false, + "showField": false, + "nonEditableReason": { + "reason": "PLUGIN_LICENSE_ERROR", + "message": "Portfolio for Jira must be licensed for the Parent Link to be available." + } + }, + "customfield_10019": null, + "aggregatetimeoriginalestimate": null, + "timeestimate": null, + "versions": [], + "issuelinks": [], + "assignee": null, + "updated": "2019-02-15T19:01:52.971-0800", + "status": { + "self": "https://some-instance-test.atlassian.net/rest/api/2/status/10001", + "description": "", + "iconUrl": "https://some-instance-test.atlassian.net/", + "name": "To Do", + "id": "10001", + "statusCategory": { + "self": "https://some-instance-test.atlassian.net/rest/api/2/statuscategory/2", + "id": 2, + "key": "new", + "colorName": "blue-gray", + "name": "New" + } + }, + "components": [ + { + "self": "https://some-instance-test.atlassian.net/rest/api/2/component/10000", + "id": "10000", + "name": "COMP-1", + "description": "Component-1" + } + ], + "timeoriginalestimate": null, + "description": "Unit test description, not that long", + "customfield_10010": null, + "customfield_10014": null, + "customfield_10015": null, + "timetracking": {}, + "customfield_10005": null, + "customfield_10006": null, + "security": "10001", + "customfield_10007": null, + "customfield_10008": null, + "attachment": [], + "customfield_10009": null, + "aggregatetimeestimate": null, + "summary": "Unit test summary", + "creator": { + "self": "https://some-instance-test.atlassian.net/rest/api/2/user?accountId=5c5f880629be9642ba529340", + "name": "admin", + "key": "admin", + "accountId": "5c5f880629be9642ba529340", + "emailAddress": "some-instance-test@gmail.com", + "avatarUrls": { + "48x48": "https://avatar-cdn.atlassian.com/d991bc281c0c0ecb0bbb2db3979ddaff?s=48&d=https%3A%2F%2Fsecure.gravatar.com%2Favatar%2Fd991bc281c0c0ecb0bbb2db3979ddaff%3Fd%3Dmm%26s%3D48%26noRedirect%3Dtrue", + "24x24": "https://avatar-cdn.atlassian.com/d991bc281c0c0ecb0bbb2db3979ddaff?s=24&d=https%3A%2F%2Fsecure.gravatar.com%2Favatar%2Fd991bc281c0c0ecb0bbb2db3979ddaff%3Fd%3Dmm%26s%3D24%26noRedirect%3Dtrue", + "16x16": "https://avatar-cdn.atlassian.com/d991bc281c0c0ecb0bbb2db3979ddaff?s=16&d=https%3A%2F%2Fsecure.gravatar.com%2Favatar%2Fd991bc281c0c0ecb0bbb2db3979ddaff%3Fd%3Dmm%26s%3D16%26noRedirect%3Dtrue", + "32x32": "https://avatar-cdn.atlassian.com/d991bc281c0c0ecb0bbb2db3979ddaff?s=32&d=https%3A%2F%2Fsecure.gravatar.com%2Favatar%2Fd991bc281c0c0ecb0bbb2db3979ddaff%3Fd%3Dmm%26s%3D32%26noRedirect%3Dtrue" + }, + "displayName": "Test User", + "active": true, + "timeZone": "America/Los_Angeles" + }, + "subtasks": [], + "reporter": { + "self": "https://some-instance-test.atlassian.net/rest/api/2/user?accountId=5c5f880629be9642ba529340", + "name": "admin", + "key": "admin", + "accountId": "5c5f880629be9642ba529340", + "emailAddress": "some-instance-test@gmail.com", + "avatarUrls": { + "48x48": "https://avatar-cdn.atlassian.com/d991bc281c0c0ecb0bbb2db3979ddaff?s=48&d=https%3A%2F%2Fsecure.gravatar.com%2Favatar%2Fd991bc281c0c0ecb0bbb2db3979ddaff%3Fd%3Dmm%26s%3D48%26noRedirect%3Dtrue", + "24x24": "https://avatar-cdn.atlassian.com/d991bc281c0c0ecb0bbb2db3979ddaff?s=24&d=https%3A%2F%2Fsecure.gravatar.com%2Favatar%2Fd991bc281c0c0ecb0bbb2db3979ddaff%3Fd%3Dmm%26s%3D24%26noRedirect%3Dtrue", + "16x16": "https://avatar-cdn.atlassian.com/d991bc281c0c0ecb0bbb2db3979ddaff?s=16&d=https%3A%2F%2Fsecure.gravatar.com%2Favatar%2Fd991bc281c0c0ecb0bbb2db3979ddaff%3Fd%3Dmm%26s%3D16%26noRedirect%3Dtrue", + "32x32": "https://avatar-cdn.atlassian.com/d991bc281c0c0ecb0bbb2db3979ddaff?s=32&d=https%3A%2F%2Fsecure.gravatar.com%2Favatar%2Fd991bc281c0c0ecb0bbb2db3979ddaff%3Fd%3Dmm%26s%3D32%26noRedirect%3Dtrue" + }, + "displayName": "Test User", + "active": true, + "timeZone": "America/Los_Angeles" + }, + "customfield_10000": "{}", + "aggregateprogress": { + "progress": 0, + "total": 0 + }, + "customfield_10001": null, + "customfield_10002": null, + "customfield_10003": null, + "customfield_10004": null, + "environment": null, + "duedate": null, + "progress": { + "progress": 0, + "total": 0 + }, + "votes": { + "self": "https://some-instance-test.atlassian.net/rest/api/2/issue/TES-41/votes", + "votes": 0, + "hasVoted": false + } + } + }, + "changelog": { + "id": "10222", + "items": [ + { + "field": "description", + "fieldtype": "jira", + "fieldId": "description", + "from": null, + "fromString": null, + "to": null, + "toString": "Unit test description, not that long" + }, + { + "field": "priority", + "fieldtype": "jira", + "fieldId": "priority", + "from": null, + "fromString": null, + "to": "2", + "toString": "High" + }, + { + "field": "reporter", + "fieldtype": "jira", + "fieldId": "reporter", + "from": null, + "fromString": null, + "to": "admin", + "toString": "Test User" + }, + { + "field": "Status", + "fieldtype": "jira", + "fieldId": "status", + "from": null, + "fromString": null, + "to": "10001", + "toString": "To Do" + }, + { + "field": "summary", + "fieldtype": "jira", + "fieldId": "summary", + "from": null, + "fromString": null, + "to": null, + "toString": "Unit test summary" + } + ] + } +} From 68659836027bf1065a8aa27fe31d9fff91eb383d Mon Sep 17 00:00:00 2001 From: Michael Kochell <6913320+mickmister@users.noreply.github.com> Date: Wed, 18 May 2022 15:58:19 -0400 Subject: [PATCH 04/15] fix ci withL npm install --verbose --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index a648ad337..2bb78c1b6 100644 --- a/Makefile +++ b/Makefile @@ -65,7 +65,7 @@ endif ## Ensures NPM dependencies are installed without having to run this all the time. webapp/.npminstall: ifneq ($(HAS_WEBAPP),) - cd webapp && $(NPM) install + cd webapp && $(NPM) install --verbose touch $@ endif From fffbadbd90bdb85253cf6721057571766b5a5d56 Mon Sep 17 00:00:00 2001 From: Michael Kochell <6913320+mickmister@users.noreply.github.com> Date: Mon, 25 Jul 2022 19:18:23 -0400 Subject: [PATCH 05/15] enforce empty during webhook call if exclude is being used --- server/subscribe.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/server/subscribe.go b/server/subscribe.go index 5f07b0a64..3d0e5b3e8 100644 --- a/server/subscribe.go +++ b/server/subscribe.go @@ -154,23 +154,28 @@ func (p *Plugin) matchesSubsciptionFilters(wh *webhook, filters SubscriptionFilt containsSecurityLevelFilter := false for _, field := range filters.Fields { + inclusion := field.Inclusion + // Broken filter, values must be provided - if field.Inclusion == "" || (field.Values.Len() == 0 && field.Inclusion != FilterEmpty) { + if inclusion == "" || (field.Values.Len() == 0 && inclusion != FilterEmpty) { return false } if field.Key == securityLevelField { containsSecurityLevelFilter = true + if inclusion == FilterExcludeAny { + inclusion = FilterEmpty + } } value := getIssueFieldValue(issue, field.Key) containsAny := value.ContainsAny(field.Values.Elems()...) containsAll := value.ContainsAll(field.Values.Elems()...) - if (field.Inclusion == FilterIncludeAny && !containsAny) || - (field.Inclusion == FilterIncludeAll && !containsAll) || - (field.Inclusion == FilterExcludeAny && containsAny) || - (field.Inclusion == FilterEmpty && value.Len() > 0) { + if (inclusion == FilterIncludeAny && !containsAny) || + (inclusion == FilterIncludeAll && !containsAll) || + (inclusion == FilterExcludeAny && containsAny) || + (inclusion == FilterEmpty && value.Len() > 0) { return false } } From a71d576b6e40f511dbe633e9d2bc553d7b09f137 Mon Sep 17 00:00:00 2001 From: Michael Kochell <6913320+mickmister@users.noreply.github.com> Date: Thu, 18 Aug 2022 04:07:24 -0400 Subject: [PATCH 06/15] add plugin setting to toggle security level functionality --- plugin.json | 8 ++++++++ server/plugin.go | 3 +++ server/subscribe.go | 8 +++++--- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/plugin.json b/plugin.json index d1c89eef3..9e2b3b0bf 100644 --- a/plugin.json +++ b/plugin.json @@ -76,6 +76,14 @@ "placeholder": "", "default": "" }, + { + "key": "SecurityLevelEmptyForJiraSubscriptions", + "display_name": "When enabled, a subscription without security level rules will filter out an issue that has a security level assigned:", + "type": "text", + "help_text": "", + "placeholder": "", + "default": true + }, { "key": "JiraAdminAdditionalHelpText", "display_name": "Additional Help Text to be shown with Jira Help:", diff --git a/server/plugin.go b/server/plugin.go index 571525556..84540f871 100644 --- a/server/plugin.go +++ b/server/plugin.go @@ -71,6 +71,9 @@ type externalConfig struct { // Additional Help Text to be shown in the output of '/jira help' command JiraAdminAdditionalHelpText string + // When enabled, a subscription without security level rules will filter out an issue that has a security level assigned + SecurityLevelEmptyForJiraSubscriptions bool + // Hide issue descriptions and comments in Webhook and Subscription messages HideDecriptionComment bool diff --git a/server/subscribe.go b/server/subscribe.go index 3d0e5b3e8..6571a8c49 100644 --- a/server/subscribe.go +++ b/server/subscribe.go @@ -153,6 +153,7 @@ func (p *Plugin) matchesSubsciptionFilters(wh *webhook, filters SubscriptionFilt } containsSecurityLevelFilter := false + useEmptySecurityLevel := p.getConfig().SecurityLevelEmptyForJiraSubscriptions for _, field := range filters.Fields { inclusion := field.Inclusion @@ -163,7 +164,7 @@ func (p *Plugin) matchesSubsciptionFilters(wh *webhook, filters SubscriptionFilt if field.Key == securityLevelField { containsSecurityLevelFilter = true - if inclusion == FilterExcludeAny { + if inclusion == FilterExcludeAny && useEmptySecurityLevel { inclusion = FilterEmpty } } @@ -180,7 +181,7 @@ func (p *Plugin) matchesSubsciptionFilters(wh *webhook, filters SubscriptionFilt } } - if !containsSecurityLevelFilter { + if !containsSecurityLevelFilter && useEmptySecurityLevel { securityLevel := getIssueFieldValue(issue, securityLevelField) if securityLevel.Len() > 0 { return false @@ -317,6 +318,7 @@ func (p *Plugin) validateSubscription(instanceID types.ID, subscription *Channel projectKey := subscription.Filters.Projects.Elems()[0] var securityLevels StringSet + useEmptySecurityLevel := p.getConfig().SecurityLevelEmptyForJiraSubscriptions for _, field := range subscription.Filters.Fields { if field.Key != securityLevelField { continue @@ -326,7 +328,7 @@ func (p *Plugin) validateSubscription(instanceID types.ID, subscription *Channel continue } - if field.Inclusion == FilterExcludeAny { + if field.Inclusion == FilterExcludeAny && useEmptySecurityLevel { return errors.New("security level does not allow for an \"Exclude\" clause") } From ce03bfffbbfdaf1a379f184253235464173970ee Mon Sep 17 00:00:00 2001 From: Michael Kochell <6913320+mickmister@users.noreply.github.com> Date: Thu, 15 Sep 2022 12:36:07 -0400 Subject: [PATCH 07/15] fix tests --- server/subscribe_test.go | 56 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 51 insertions(+), 5 deletions(-) diff --git a/server/subscribe_test.go b/server/subscribe_test.go index d3430adf9..889a22192 100644 --- a/server/subscribe_test.go +++ b/server/subscribe_test.go @@ -28,8 +28,9 @@ func TestValidateSubscription(t *testing.T) { p.SetAPI(api) for name, tc := range map[string]struct { - subscription *ChannelSubscription - errorMessage string + subscription *ChannelSubscription + errorMessage string + disableSecurityConfig bool }{ "no event selected": { subscription: &ChannelSubscription{ @@ -129,6 +130,28 @@ func TestValidateSubscription(t *testing.T) { }, errorMessage: "security level does not allow for an \"Exclude\" clause", }, + "security config disabled, valid 'Exclude' of security level": { + subscription: &ChannelSubscription{ + ID: "id", + Name: "name", + ChannelID: "channelid", + InstanceID: "instance_id", + Filters: SubscriptionFilters{ + Events: NewStringSet("issue_created"), + Projects: NewStringSet("TEST"), + IssueTypes: NewStringSet("10001"), + Fields: []FieldFilter{ + { + Key: "security", + Inclusion: FilterExcludeAny, + Values: NewStringSet("10001"), + }, + }, + }, + }, + disableSecurityConfig: true, + errorMessage: "", + }, "invalid access to security level": { subscription: &ChannelSubscription{ ID: "id", @@ -171,6 +194,10 @@ func TestValidateSubscription(t *testing.T) { api.On("KVGet", testSubKey).Return(nil, nil) + p.updateConfig(func(conf *config) { + conf.SecurityLevelEmptyForJiraSubscriptions = !tc.disableSecurityConfig + }) + client := testClient{} err := p.validateSubscription(testInstance1.InstanceID, tc.subscription, client) @@ -440,9 +467,10 @@ func TestGetChannelsSubscribed(t *testing.T) { p.instanceStore = p.getMockInstanceStoreKV(0) for name, tc := range map[string]struct { - WebhookTestData string - Subs *Subscriptions - ChannelSubscriptions []ChannelSubscription + WebhookTestData string + Subs *Subscriptions + ChannelSubscriptions []ChannelSubscription + disableSecurityConfig bool }{ "no filters selected": { WebhookTestData: "webhook-issue-created.json", @@ -1538,6 +1566,23 @@ func TestGetChannelsSubscribed(t *testing.T) { }), ChannelSubscriptions: []ChannelSubscription{}, }, + "security config disabled, no security level provided in subscription, but security level is present in issue": { + WebhookTestData: "webhook-issue-created-with-security-level.json", + Subs: withExistingChannelSubscriptions([]ChannelSubscription{ + { + ID: "rg86cd65efdjdjezgisgxaitzh", + ChannelID: "sampleChannelId", + Filters: SubscriptionFilters{ + Events: NewStringSet("event_created"), + Projects: NewStringSet("TES"), + IssueTypes: NewStringSet("10001"), + Fields: []FieldFilter{}, + }, + }, + }), + ChannelSubscriptions: []ChannelSubscription{{ChannelID: "sampleChannelId"}}, + disableSecurityConfig: true, + }, "security level provided in subscription, but different security level is present in issue": { WebhookTestData: "webhook-issue-created-with-security-level.json", Subs: withExistingChannelSubscriptions([]ChannelSubscription{ @@ -1588,6 +1633,7 @@ func TestGetChannelsSubscribed(t *testing.T) { p.updateConfig(func(conf *config) { conf.Secret = someSecret + conf.SecurityLevelEmptyForJiraSubscriptions = !tc.disableSecurityConfig }) p.SetAPI(api) From c64f1b56af1f94c6ff9264e0e0315cfbc3110a5e Mon Sep 17 00:00:00 2001 From: Michael Kochell <6913320+mickmister@users.noreply.github.com> Date: Tue, 6 Dec 2022 18:23:33 -0500 Subject: [PATCH 08/15] add form validation for securitylevel field --- server/user.go | 7 ++- ...channel_subscription_filters.test.tsx.snap | 1 + .../edit_channel_subscription.test.tsx.snap | 1 + .../channel_subscription_filter.test.tsx | 58 +++++++++++++++++++ .../channel_subscription_filter.tsx | 33 +++++++++-- .../channel_subscription_filters.test.tsx | 1 + .../channel_subscription_filters.tsx | 4 +- .../edit_channel_subscription.test.tsx | 9 +-- .../edit_channel_subscription.tsx | 1 + .../modals/channel_subscriptions/index.ts | 5 +- .../channel_subscriptions/shared_props.ts | 1 + webapp/src/utils/jira_issue_metadata.tsx | 4 ++ 12 files changed, 113 insertions(+), 12 deletions(-) diff --git a/server/user.go b/server/user.go index e25d5b1c5..9b3512bd8 100644 --- a/server/user.go +++ b/server/user.go @@ -224,10 +224,13 @@ func (p *Plugin) httpGetSettingsInfo(w http.ResponseWriter, r *http.Request) (in errors.New("not authorized")) } + conf := p.getConfig() return respondJSON(w, struct { - UIEnabled bool `json:"ui_enabled"` + UIEnabled bool `json:"ui_enabled"` + SecurityLevelEmptyForJiraSubscriptions bool `json:"security_level_empty_for_jira_subscriptions"` }{ - UIEnabled: p.getConfig().EnableJiraUI, + UIEnabled: conf.EnableJiraUI, + SecurityLevelEmptyForJiraSubscriptions: conf.SecurityLevelEmptyForJiraSubscriptions, }) } diff --git a/webapp/src/components/modals/channel_subscriptions/__snapshots__/channel_subscription_filters.test.tsx.snap b/webapp/src/components/modals/channel_subscriptions/__snapshots__/channel_subscription_filters.test.tsx.snap index ef1cfd844..a33535f58 100644 --- a/webapp/src/components/modals/channel_subscriptions/__snapshots__/channel_subscription_filters.test.tsx.snap +++ b/webapp/src/components/modals/channel_subscriptions/__snapshots__/channel_subscription_filters.test.tsx.snap @@ -155,6 +155,7 @@ exports[`components/ChannelSubscriptionFilters should match snapshot 1`] = ` onChange={[Function]} removeFilter={[Function]} removeValidate={[MockFunction]} + securityLevelEmptyForJiraSubscriptions={true} theme={Object {}} value={ Object { diff --git a/webapp/src/components/modals/channel_subscriptions/__snapshots__/edit_channel_subscription.test.tsx.snap b/webapp/src/components/modals/channel_subscriptions/__snapshots__/edit_channel_subscription.test.tsx.snap index 9ff6146e3..048a7355c 100644 --- a/webapp/src/components/modals/channel_subscriptions/__snapshots__/edit_channel_subscription.test.tsx.snap +++ b/webapp/src/components/modals/channel_subscriptions/__snapshots__/edit_channel_subscription.test.tsx.snap @@ -3544,6 +3544,7 @@ exports[`components/EditChannelSubscription should match snapshot after fetching } onChange={[Function]} removeValidate={[Function]} + securityLevelEmptyForJiraSubscriptions={true} theme={ Object { "awayIndicator": "#ffbc42", diff --git a/webapp/src/components/modals/channel_subscriptions/channel_subscription_filter.test.tsx b/webapp/src/components/modals/channel_subscriptions/channel_subscription_filter.test.tsx index 56da56f7b..64c4c7157 100644 --- a/webapp/src/components/modals/channel_subscriptions/channel_subscription_filter.test.tsx +++ b/webapp/src/components/modals/channel_subscriptions/channel_subscription_filter.test.tsx @@ -31,6 +31,7 @@ describe('components/ChannelSubscriptionFilter', () => { onChange: jest.fn(), removeFilter: jest.fn(), instanceID: 'https://something.atlassian.net', + securityLevelEmptyForJiraSubscriptions: true, }; test('should match snapshot', () => { @@ -116,4 +117,61 @@ describe('components/ChannelSubscriptionFilter', () => { result = wrapper.instance().checkFieldConflictError(); expect(result).toEqual('FieldName does not exist for issue type(s): Task.'); }); + + test('checkInclusionError should return an error string when there is an invalid inclusion value', () => { + const props: Props = { + ...baseProps, + field: { + ...baseProps.field, + schema: { + ...baseProps.field.schema, + type: 'securitylevel', + }, + }, + }; + const wrapper = shallow( + + ); + + let isValid; + isValid = wrapper.instance().isValid(); + expect(isValid).toBe(true); + + wrapper.setProps({ + ...props, + value: { + inclusion: FilterFieldInclusion.EMPTY, + key: 'securitylevel', + values: [], + }, + }); + + isValid = wrapper.instance().isValid(); + expect(isValid).toBe(true); + + wrapper.setProps({ + ...props, + value: { + inclusion: FilterFieldInclusion.INCLUDE_ANY, + key: 'securitylevel', + values: [], + }, + }); + + isValid = wrapper.instance().isValid(); + expect(isValid).toBe(true); + + wrapper.setProps({ + ...props, + value: { + inclusion: FilterFieldInclusion.EXCLUDE_ANY, + key: 'securitylevel', + values: [], + }, + }); + + isValid = wrapper.instance().isValid(); + expect(isValid).toBe(false); + expect(wrapper.find('.error-text').text()).toEqual('Security level inclusion cannot be "Exclude Any"'); + }); }); diff --git a/webapp/src/components/modals/channel_subscriptions/channel_subscription_filter.tsx b/webapp/src/components/modals/channel_subscriptions/channel_subscription_filter.tsx index 9abdf12e6..38369c221 100644 --- a/webapp/src/components/modals/channel_subscriptions/channel_subscription_filter.tsx +++ b/webapp/src/components/modals/channel_subscriptions/channel_subscription_filter.tsx @@ -5,7 +5,7 @@ import {Theme} from 'mattermost-redux/types/preferences'; import ReactSelectSetting from 'components/react_select_setting'; import JiraEpicSelector from 'components/data_selectors/jira_epic_selector'; -import {isEpicLinkField, isMultiSelectField, isLabelField} from 'utils/jira_issue_metadata'; +import {isEpicLinkField, isMultiSelectField, isLabelField, isSecurityLevelField} from 'utils/jira_issue_metadata'; import {FilterField, FilterValue, ReactSelectOption, IssueMetadata, IssueType, FilterFieldInclusion} from 'types/model'; import ConfirmModal from 'components/confirm_modal'; import JiraAutoCompleteSelector from 'components/data_selectors/jira_autocomplete_selector'; @@ -22,6 +22,7 @@ export type Props = { addValidate: (isValid: () => boolean) => void; removeValidate: (isValid: () => boolean) => void; instanceID: string; + securityLevelEmptyForJiraSubscriptions: boolean; }; export type State = { @@ -103,7 +104,13 @@ export default class ChannelSubscriptionFilter extends React.PureComponent { - const error = this.checkFieldConflictError(); + let error = this.checkFieldConflictError(); + if (error) { + this.setState({error}); + return false; + } + + error = this.checkInclusionError(); if (error) { this.setState({error}); return false; @@ -112,6 +119,16 @@ export default class ChannelSubscriptionFilter extends React.PureComponent { + const inclusion = this.props.value && this.props.value.inclusion; + + if (isSecurityLevelField(this.props.field) && inclusion === FilterFieldInclusion.EXCLUDE_ANY && this.props.securityLevelEmptyForJiraSubscriptions) { + return 'Security level inclusion cannot be "Exclude Any"'; + } + + return null; + } + checkFieldConflictError = (): string | null => { const conflictIssueTypes = this.getConflictingIssueTypes().map((it) => it.name); if (conflictIssueTypes.length) { @@ -167,13 +184,21 @@ export default class ChannelSubscriptionFilter extends React.PureComponent opt.value === FilterFieldInclusion.INCLUDE_ALL); inclusionSelectOptions.splice(includeAllIndex, 1); @@ -296,7 +321,7 @@ export default class ChannelSubscriptionFilter extends React.PureComponent - {this.checkFieldConflictError()} + {this.state.error}
diff --git a/webapp/src/components/modals/channel_subscriptions/channel_subscription_filters.test.tsx b/webapp/src/components/modals/channel_subscriptions/channel_subscription_filters.test.tsx index ec59a9796..f3f1b5df1 100644 --- a/webapp/src/components/modals/channel_subscriptions/channel_subscription_filters.test.tsx +++ b/webapp/src/components/modals/channel_subscriptions/channel_subscription_filters.test.tsx @@ -58,6 +58,7 @@ describe('components/ChannelSubscriptionFilters', () => { removeValidate: jest.fn(), onChange: jest.fn(), instanceID: 'https://something.atlassian.net', + securityLevelEmptyForJiraSubscriptions: true, }; test('should match snapshot', () => { diff --git a/webapp/src/components/modals/channel_subscriptions/channel_subscription_filters.tsx b/webapp/src/components/modals/channel_subscriptions/channel_subscription_filters.tsx index 8ff79f682..ee75cc00b 100644 --- a/webapp/src/components/modals/channel_subscriptions/channel_subscription_filters.tsx +++ b/webapp/src/components/modals/channel_subscriptions/channel_subscription_filters.tsx @@ -16,6 +16,7 @@ export type Props = { removeValidate: (isValid: () => boolean) => void; onChange: (f: FilterValue[]) => void; instanceID: string; + securityLevelEmptyForJiraSubscriptions: boolean; }; type State = { @@ -32,7 +33,7 @@ export default class ChannelSubscriptionFilters extends React.PureComponent f === oldValue); if (index === -1) { - newValues.push({inclusion: FilterFieldInclusion.INCLUDE_ANY, values: [], ...newValue}); + newValues.push({...newValue, inclusion: FilterFieldInclusion.INCLUDE_ANY, values: []}); this.setState({showCreateRow: false}); } else { newValues.splice(index, 1, newValue); @@ -104,6 +105,7 @@ export default class ChannelSubscriptionFilters extends React.PureComponent
); diff --git a/webapp/src/components/modals/channel_subscriptions/edit_channel_subscription.test.tsx b/webapp/src/components/modals/channel_subscriptions/edit_channel_subscription.test.tsx index b2a920a6c..b7fe1b67f 100644 --- a/webapp/src/components/modals/channel_subscriptions/edit_channel_subscription.test.tsx +++ b/webapp/src/components/modals/channel_subscriptions/edit_channel_subscription.test.tsx @@ -6,7 +6,6 @@ import {shallow} from 'enzyme'; import Preferences from 'mattermost-redux/constants/preferences'; -import cloudProjectMetadata from 'testdata/cloud-get-jira-project-metadata.json'; import cloudIssueMetadata from 'testdata/cloud-get-create-issue-metadata-for-project.json'; import serverProjectMetadata from 'testdata/server-get-jira-project-metadata.json'; import serverIssueMetadata from 'testdata/server-get-create-issue-metadata-for-project-many-fields.json'; @@ -14,7 +13,8 @@ import testChannel from 'testdata/channel.json'; import {IssueMetadata, ProjectMetadata, FilterFieldInclusion} from 'types/model'; -import EditChannelSubscription from './edit_channel_subscription'; +import EditChannelSubscription, {Props} from './edit_channel_subscription'; +import {Channel} from 'mattermost-redux/types/channels'; describe('components/EditChannelSubscription', () => { const baseActions = { @@ -71,15 +71,16 @@ describe('components/EditChannelSubscription', () => { instance_id: 'https://something.atlassian.net', }; - const baseProps = { + const baseProps: Props = { ...baseActions, - channel: testChannel, + channel: testChannel as unknown as Channel, theme: Preferences.THEMES.default, finishEditSubscription: jest.fn(), channelSubscriptions: [channelSubscriptionForCloud], close: jest.fn(), selectedSubscription: channelSubscriptionForCloud, creatingSubscription: false, + securityLevelEmptyForJiraSubscriptions: true, }; const baseState = { diff --git a/webapp/src/components/modals/channel_subscriptions/edit_channel_subscription.tsx b/webapp/src/components/modals/channel_subscriptions/edit_channel_subscription.tsx index cbd914ba5..22e1b5822 100644 --- a/webapp/src/components/modals/channel_subscriptions/edit_channel_subscription.tsx +++ b/webapp/src/components/modals/channel_subscriptions/edit_channel_subscription.tsx @@ -392,6 +392,7 @@ export default class EditChannelSubscription extends PureComponent addValidate={this.validator.addComponent} removeValidate={this.validator.removeComponent} instanceID={this.state.instanceID} + securityLevelEmptyForJiraSubscriptions={this.props.securityLevelEmptyForJiraSubscriptions} />
); } From 1d5d8d0878e9d53d43a2927d988ceead60ff8040 Mon Sep 17 00:00:00 2001 From: Michael Kochell <6913320+mickmister@users.noreply.github.com> Date: Thu, 2 Feb 2023 02:02:10 -0500 Subject: [PATCH 12/15] show message about security level under JQL query --- server/subscribe.go | 3 ++- .../channel_subscription_filters.tsx | 20 ------------------- .../edit_channel_subscription.tsx | 19 +++++++++++++++++- webapp/src/utils/jira_issue_metadata.tsx | 18 +++++++++++++++-- webapp/src/utils/styles.ts | 1 + 5 files changed, 37 insertions(+), 24 deletions(-) diff --git a/server/subscribe.go b/server/subscribe.go index e08a4c012..754f75db0 100644 --- a/server/subscribe.go +++ b/server/subscribe.go @@ -15,6 +15,7 @@ import ( jira "github.com/andygrunwald/go-jira" "github.com/pkg/errors" + "github.com/trivago/tgo/tcontainer" "github.com/mattermost/mattermost-server/v6/model" @@ -367,7 +368,7 @@ func (p *Plugin) validateSubscription(instanceID types.ID, subscription *Channel } func getSecurityLevelsForProject(client Client, projectKey string) ([]string, error) { - createMeta, err := client.GetCreateMeta(&jira.GetQueryOptions{ + createMeta, err := client.GetCreateMetaInfo(&jira.GetQueryOptions{ Expand: "projects.issuetypes.fields", ProjectKeys: projectKey, }) diff --git a/webapp/src/components/modals/channel_subscriptions/channel_subscription_filters.tsx b/webapp/src/components/modals/channel_subscriptions/channel_subscription_filters.tsx index beee5bc7a..ee75cc00b 100644 --- a/webapp/src/components/modals/channel_subscriptions/channel_subscription_filters.tsx +++ b/webapp/src/components/modals/channel_subscriptions/channel_subscription_filters.tsx @@ -64,20 +64,6 @@ export default class ChannelSubscriptionFilters extends React.PureComponent { - if (!this.props.securityLevelEmptyForJiraSubscriptions) { - return false; - } - - for (const v of this.props.values) { - if (v.key === 'securitylevel') { - return false; - } - } - - return true; - } - render(): JSX.Element { const {fields, values} = this.props; const {showCreateRow} = this.state; @@ -92,11 +78,6 @@ export default class ChannelSubscriptionFilters extends React.PureComponent conf.field.key === f.key); }); - let securityLevelMessage = ''; - if (this.shouldShowSecurityLevelMessage()) { - securityLevelMessage = 'Note that since you have not selected a security level filter, the subscription will only allow issues that have no security level assigned'; - } - return (
- {securityLevelMessage} ); } diff --git a/webapp/src/components/modals/channel_subscriptions/edit_channel_subscription.tsx b/webapp/src/components/modals/channel_subscriptions/edit_channel_subscription.tsx index 22e1b5822..f8163d75f 100644 --- a/webapp/src/components/modals/channel_subscriptions/edit_channel_subscription.tsx +++ b/webapp/src/components/modals/channel_subscriptions/edit_channel_subscription.tsx @@ -19,6 +19,7 @@ import { getConflictingFields, generateJQLStringFromSubscriptionFilters, getIssueTypes, + filterValueIsSecurityField, } from 'utils/jira_issue_metadata'; import {ChannelSubscription, ChannelSubscriptionFilters as ChannelSubscriptionFiltersModel, ReactSelectOption, FilterValue, IssueMetadata} from 'types/model'; @@ -172,6 +173,14 @@ export default class EditChannelSubscription extends PureComponent this.setState({conflictingError: null}); } + shouldShowEmptySecurityLevelMessage = (): boolean => { + if (!this.props.securityLevelEmptyForJiraSubscriptions) { + return false; + } + + return !this.state.filters.fields.some(filterValueIsSecurityField); + } + handleIssueChange = (id: keyof ChannelSubscriptionFiltersModel, value: string[] | null) => { const finalValue = value || []; const filters = {...this.state.filters, issue_types: finalValue}; @@ -399,8 +408,16 @@ export default class EditChannelSubscription extends PureComponent {'Approximate JQL Output'}
- {generateJQLStringFromSubscriptionFilters(this.state.jiraIssueMetadata, filterFields, this.state.filters)} + {generateJQLStringFromSubscriptionFilters(this.state.jiraIssueMetadata, filterFields, this.state.filters, this.props.securityLevelEmptyForJiraSubscriptions)}
+ {this.shouldShowEmptySecurityLevelMessage() && ( +
+ + {'Note'} + {' that since you have not selected a security level filter, the subscription will only allow issues that have no security level assigned.'} + +
+ )} ); diff --git a/webapp/src/utils/jira_issue_metadata.tsx b/webapp/src/utils/jira_issue_metadata.tsx index 849bf1374..05ccd7c31 100644 --- a/webapp/src/utils/jira_issue_metadata.tsx +++ b/webapp/src/utils/jira_issue_metadata.tsx @@ -8,12 +8,14 @@ import { IssueType, JiraField, FilterField, + FilterValue, SelectField, StringArrayField, IssueTypeIdentifier, ChannelSubscriptionFilters, FilterFieldInclusion, JiraFieldCustomTypeEnums, + JiraFieldTypeEnums, } from 'types/model'; type FieldWithInfo = JiraField & { @@ -280,6 +282,10 @@ export function isSecurityLevelField(field: JiraField | FilterField): boolean { return field.schema.type === 'securitylevel'; } +export function filterValueIsSecurityField(value: FilterValue): boolean { + return value.key === JiraFieldTypeEnums.SECURITY; +} + // Some Jira fields have special names for JQL function getFieldNameForJQL(field: FilterField) { switch (field.key) { @@ -300,7 +306,7 @@ function quoteGuard(s: string) { return s; } -export function generateJQLStringFromSubscriptionFilters(issueMetadata: IssueMetadata, fields: FilterField[], filters: ChannelSubscriptionFilters) { +export function generateJQLStringFromSubscriptionFilters(issueMetadata: IssueMetadata, fields: FilterField[], filters: ChannelSubscriptionFilters, securityLevelEmptyForJiraSubscriptions: boolean) { const projectJQL = `Project = ${quoteGuard(filters.projects[0]) || '?'}`; let issueTypeValueString = '?'; @@ -317,7 +323,7 @@ export function generateJQLStringFromSubscriptionFilters(issueMetadata: IssueMet } const issueTypesJQL = `IssueType IN ${issueTypeValueString}`; - const filterFieldsJQL = filters.fields.map(({key, inclusion, values}): string => { + let filterFieldsJQL = filters.fields.map(({key, inclusion, values}): string => { const field = fields.find((f) => f.key === key); if (!field) { // broken filter @@ -358,5 +364,13 @@ export function generateJQLStringFromSubscriptionFilters(issueMetadata: IssueMet return `${quoteGuard(fieldName)} ${inclusionString} ${valueString}`; }).join(' AND '); + const shouldShowEmptySecurityLevel = securityLevelEmptyForJiraSubscriptions && !filters.fields.some(filterValueIsSecurityField); + if (shouldShowEmptySecurityLevel) { + if (filterFieldsJQL.length) { + filterFieldsJQL += ' AND '; + } + filterFieldsJQL += '"Security Level" IS EMPTY'; + } + return [projectJQL, issueTypesJQL, filterFieldsJQL].filter(Boolean).join(' AND '); } diff --git a/webapp/src/utils/styles.ts b/webapp/src/utils/styles.ts index ab3284c39..2f0744f7c 100644 --- a/webapp/src/utils/styles.ts +++ b/webapp/src/utils/styles.ts @@ -12,6 +12,7 @@ export const getBaseStyles = (theme: Theme) => { background: changeOpacity(theme.centerChannelColor, 0.08), borderRadius: '4px', marginTop: '8px', + marginBottom: '8px', fontSize: '13px', }), }; From 7811c978214c1a469a67a84aded3cab0de4e8796 Mon Sep 17 00:00:00 2001 From: Michael Kochell <6913320+mickmister@users.noreply.github.com> Date: Thu, 23 Feb 2023 15:19:05 -0500 Subject: [PATCH 13/15] extract if statement into function --- server/issue_test.go | 2 +- server/subscribe.go | 23 ++++++++++++++++------- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/server/issue_test.go b/server/issue_test.go index ddefc901c..a01f42d7b 100644 --- a/server/issue_test.go +++ b/server/issue_test.go @@ -85,7 +85,7 @@ func (client testClient) AddComment(issueKey string, comment *jira.Comment) (*ji return nil, nil } -func (client testClient) GetCreateMeta(options *jira.GetQueryOptions) (*jira.CreateMetaInfo, error) { +func (client testClient) GetCreateMetaInfo(options *jira.GetQueryOptions) (*jira.CreateMetaInfo, error) { return &jira.CreateMetaInfo{ Projects: []*jira.MetaProject{ { diff --git a/server/subscribe.go b/server/subscribe.go index 754f75db0..4959b56f3 100644 --- a/server/subscribe.go +++ b/server/subscribe.go @@ -171,13 +171,7 @@ func (p *Plugin) matchesSubsciptionFilters(wh *webhook, filters SubscriptionFilt } value := getIssueFieldValue(issue, field.Key) - containsAny := value.ContainsAny(field.Values.Elems()...) - containsAll := value.ContainsAll(field.Values.Elems()...) - - if (inclusion == FilterIncludeAny && !containsAny) || - (inclusion == FilterIncludeAll && !containsAll) || - (inclusion == FilterExcludeAny && containsAny) || - (inclusion == FilterEmpty && value.Len() > 0) { + if !isValidFieldInclusion(field, value) { return false } } @@ -192,6 +186,21 @@ func (p *Plugin) matchesSubsciptionFilters(wh *webhook, filters SubscriptionFilt return true } +func isValidFieldInclusion(field FieldFilter, value StringSet) bool { + inclusion := field.Inclusion + containsAny := value.ContainsAny(field.Values.Elems()...) + containsAll := value.ContainsAll(field.Values.Elems()...) + + if (inclusion == FilterIncludeAny && !containsAny) || + (inclusion == FilterIncludeAll && !containsAll) || + (inclusion == FilterExcludeAny && containsAny) || + (inclusion == FilterEmpty && value.Len() > 0) { + return false + } + + return true +} + func (p *Plugin) getChannelsSubscribed(wh *webhook, instanceID types.ID) ([]ChannelSubscription, error) { subs, err := p.getSubscriptions(instanceID) if err != nil { From d362a7ffc9d9dccf69ce8e37f7e8c78e6925a416 Mon Sep 17 00:00:00 2001 From: Michael Kochell <6913320+mickmister@users.noreply.github.com> Date: Fri, 17 Mar 2023 17:26:07 -0400 Subject: [PATCH 14/15] fix issues from ci --- docker-compose.yml | 2 +- server/subscribe.go | 5 ++--- .../edit_channel_subscription.test.tsx.snap | 11 ++++++++++- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 0e3fbbd0c..dc874ba93 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,7 +1,7 @@ version: '3' services: jira: - image: atlassian/jira-software:7 # It is recommended to test with version 7. Most, if not all, compatibility issues between Jira Server/Cloud exist in version 7. + image: atlassian/jira-software:9 # It is recommended to test with version 7. Most, if not all, compatibility issues between Jira Server/Cloud exist in version 7. # image: atlassian/jira-software:8 # Test with version 8 as well when adding new webhook handling logic, or introducing a new Jira API call. ports: - "8080:8080" diff --git a/server/subscribe.go b/server/subscribe.go index 4959b56f3..4319b78a9 100644 --- a/server/subscribe.go +++ b/server/subscribe.go @@ -171,7 +171,7 @@ func (p *Plugin) matchesSubsciptionFilters(wh *webhook, filters SubscriptionFilt } value := getIssueFieldValue(issue, field.Key) - if !isValidFieldInclusion(field, value) { + if !isValidFieldInclusion(field, value, inclusion) { return false } } @@ -186,8 +186,7 @@ func (p *Plugin) matchesSubsciptionFilters(wh *webhook, filters SubscriptionFilt return true } -func isValidFieldInclusion(field FieldFilter, value StringSet) bool { - inclusion := field.Inclusion +func isValidFieldInclusion(field FieldFilter, value StringSet, inclusion string) bool { containsAny := value.ContainsAny(field.Values.Elems()...) containsAll := value.ContainsAll(field.Values.Elems()...) diff --git a/webapp/src/components/modals/channel_subscriptions/__snapshots__/edit_channel_subscription.test.tsx.snap b/webapp/src/components/modals/channel_subscriptions/__snapshots__/edit_channel_subscription.test.tsx.snap index 048a7355c..3b0054080 100644 --- a/webapp/src/components/modals/channel_subscriptions/__snapshots__/edit_channel_subscription.test.tsx.snap +++ b/webapp/src/components/modals/channel_subscriptions/__snapshots__/edit_channel_subscription.test.tsx.snap @@ -3612,13 +3612,22 @@ exports[`components/EditChannelSubscription should match snapshot after fetching "background": "rgba(61,60,64,0.08)", "borderRadius": "4px", "fontSize": "13px", + "marginBottom": "8px", "marginTop": "8px", "padding": "10px 12px", } } > - Project = KT AND IssueType IN (Bug) AND "MJK - Radio Buttons" IN (1) AND affectedVersion IN (d) AND "Epic Link" IN (IDT-24) + Project = KT AND IssueType IN (Bug) AND "MJK - Radio Buttons" IN (1) AND affectedVersion IN (d) AND "Epic Link" IN (IDT-24) AND "Security Level" IS EMPTY + + +
+ + + Note + + that since you have not selected a security level filter, the subscription will only allow issues that have no security level assigned.
From cd84020d225f8f161da852f433134fdee2fda82b Mon Sep 17 00:00:00 2001 From: Michael Kochell <6913320+mickmister@users.noreply.github.com> Date: Fri, 17 Mar 2023 17:53:27 -0400 Subject: [PATCH 15/15] fix merge issues and lint --- server/issue_test.go | 2 +- server/subscribe.go | 6 +++--- .../edit_channel_subscription.test.tsx | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/server/issue_test.go b/server/issue_test.go index a01f42d7b..a8feb7def 100644 --- a/server/issue_test.go +++ b/server/issue_test.go @@ -85,7 +85,7 @@ func (client testClient) AddComment(issueKey string, comment *jira.Comment) (*ji return nil, nil } -func (client testClient) GetCreateMetaInfo(options *jira.GetQueryOptions) (*jira.CreateMetaInfo, error) { +func (client testClient) GetCreateMetaInfo(api plugin.API, options *jira.GetQueryOptions) (*jira.CreateMetaInfo, error) { return &jira.CreateMetaInfo{ Projects: []*jira.MetaProject{ { diff --git a/server/subscribe.go b/server/subscribe.go index 4319b78a9..a3361ce86 100644 --- a/server/subscribe.go +++ b/server/subscribe.go @@ -342,7 +342,7 @@ func (p *Plugin) validateSubscription(instanceID types.ID, subscription *Channel } if securityLevels == nil { - securityLevelsArray, err := getSecurityLevelsForProject(client, projectKey) + securityLevelsArray, err := p.getSecurityLevelsForProject(client, projectKey) if err != nil { return errors.Wrap(err, "failed to get security levels for project") } @@ -375,8 +375,8 @@ func (p *Plugin) validateSubscription(instanceID types.ID, subscription *Channel return nil } -func getSecurityLevelsForProject(client Client, projectKey string) ([]string, error) { - createMeta, err := client.GetCreateMetaInfo(&jira.GetQueryOptions{ +func (p *Plugin) getSecurityLevelsForProject(client Client, projectKey string) ([]string, error) { + createMeta, err := client.GetCreateMetaInfo(p.API, &jira.GetQueryOptions{ Expand: "projects.issuetypes.fields", ProjectKeys: projectKey, }) diff --git a/webapp/src/components/modals/channel_subscriptions/edit_channel_subscription.test.tsx b/webapp/src/components/modals/channel_subscriptions/edit_channel_subscription.test.tsx index b7fe1b67f..f2e867680 100644 --- a/webapp/src/components/modals/channel_subscriptions/edit_channel_subscription.test.tsx +++ b/webapp/src/components/modals/channel_subscriptions/edit_channel_subscription.test.tsx @@ -5,6 +5,7 @@ import React from 'react'; import {shallow} from 'enzyme'; import Preferences from 'mattermost-redux/constants/preferences'; +import {Channel} from 'mattermost-redux/types/channels'; import cloudIssueMetadata from 'testdata/cloud-get-create-issue-metadata-for-project.json'; import serverProjectMetadata from 'testdata/server-get-jira-project-metadata.json'; @@ -14,7 +15,6 @@ import testChannel from 'testdata/channel.json'; import {IssueMetadata, ProjectMetadata, FilterFieldInclusion} from 'types/model'; import EditChannelSubscription, {Props} from './edit_channel_subscription'; -import {Channel} from 'mattermost-redux/types/channels'; describe('components/EditChannelSubscription', () => { const baseActions = {