MM-15439 refactor and merge webhooks #72 from jira2 (#82)

* MM-15439 Restores 1.0 compatibility for webhook filtering (#72)

* wip

* MM-15439: Restored webhook compatibility with 1.0

* Fixed the tests

* Fixed eventIssueDeleted->eventUresolvedIssueDeleted

* Added support for eventIssueAssigned to URL params

* PR feedback

* PR feedback: adjusted default event mask

* PR feedback from #72, minor

* Style: minor simplification, maskAll

* Refactored for testability

- Refactored webhook code, added tests
- Added separate interfaces (`UserStore`, `InstanceStore`, etc.) to KV methods
- Added store (kv) interface pointers to `type Plugin`
- Added POC mocks for some test stores (as needed)

* Added .code-workspace to .gitignore

* PR feedback: KV->Store, misc

* PR feedback: removed inner function

* PR feedback: clarify recursive URL-unescaping

.gitignore

@@ -4,5 +4,8 @@ dist
 bin
 vendor
 
+# Dev
+*.code-workspace
+
 # Mac
 .DS_Store

go.mod

@@ -9,14 +9,15 @@ require (
 	github.com/dgrijalva/jwt-go v3.2.0+incompatible
 	github.com/dyatlov/go-opengraph v0.0.0-20180429202543-816b6608b3c8 // indirect
 	github.com/fatih/structs v1.1.0 // indirect
+	github.com/go-ldap/ldap v3.0.3+incompatible // indirect
 	github.com/golang/protobuf v1.3.1 // indirect
 	github.com/google/go-querystring v0.0.0-20190318165438-c8c88dbee036 // indirect
 	github.com/google/uuid v1.1.1 // indirect
 	github.com/gorilla/websocket v1.4.0 // indirect
 	github.com/hashicorp/go-hclog v0.0.0-20180910232447-e45cbeb79f04 // indirect
 	github.com/hashicorp/go-plugin v0.0.0-20180814222501-a4620f9913d1 // indirect
 	github.com/hashicorp/yamux v0.0.0-20181012175058-2f1d1f20f75d // indirect
-	github.com/mattermost/mattermost-server v5.6.5+incompatible
+	github.com/mattermost/mattermost-server v5.10.1+incompatible
 	github.com/mattermost/viper v0.0.0-20181112161711-f99c30686b86 // indirect
 	github.com/mitchellh/go-testing-interface v1.0.0 // indirect
 	github.com/mitchellh/mapstructure v1.1.2 // indirect
@@ -43,5 +44,6 @@ require (
 	google.golang.org/appengine v1.5.0 // indirect
 	google.golang.org/genproto v0.0.0-20190404172233-64821d5d2107 // indirect
 	google.golang.org/grpc v1.20.0 // indirect
+	gopkg.in/asn1-ber.v1 v1.0.0-20181015200546-f715ec2f112d // indirect
 	gopkg.in/natefinch/lumberjack.v2 v2.0.0-20170531160350-a96e63847dc3 // indirect
 )

go.sum

@@ -20,6 +20,8 @@ github.com/fatih/structs v1.1.0 h1:Q7juDM0QtcnhCpeyLGQKyg4TOIghuNXrkL32pHAUMxo=
 github.com/fatih/structs v1.1.0/go.mod h1:9NiDSp5zOcgEDl+j00MP/WkGVPOlPRLejGD8Ga6PJ7M=
 github.com/fsnotify/fsnotify v1.4.7 h1:IXs+QLmnXW2CcXuY+8Mzv/fWEsPGWxqefPtCP5CnV9I=
 github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo=
+github.com/go-ldap/ldap v3.0.3+incompatible h1:HTeSZO8hWMS1Rgb2Ziku6b8a7qRIZZMHjsvuZyatzwk=
+github.com/go-ldap/ldap v3.0.3+incompatible/go.mod h1:qfd9rJvER9Q0/D/Sqn1DfHRoBp40uXYvFoEVrNEPqRc=
 github.com/gogo/googleapis v1.1.0/go.mod h1:gf4bu3Q80BeJ6H1S1vYPm8/ELATdvryBaNFGgqEef3s=
 github.com/gogo/protobuf v1.2.0/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ=
 github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b h1:VKtxabqXZkF25pY9ekfRL6a582T4P37/31XEstQ5p58=
@@ -47,6 +49,8 @@ github.com/magiconair/properties v1.8.0 h1:LLgXmsheXeRoUOBOjtwPQCWIYqM/LU1ayDtDe
 github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ=
 github.com/mattermost/mattermost-server v5.6.5+incompatible h1:3GOnGVeecQ8o7kAQqIERdwblc7ezJ/pSWFkVWZEprLk=
 github.com/mattermost/mattermost-server v5.6.5+incompatible/go.mod h1:5L6MjAec+XXQwMIt791Ganu45GKsSiM+I0tLR9wUj8Y=
+github.com/mattermost/mattermost-server v5.10.1+incompatible h1:bSwuCGIOuKGVCxAMsVeEnBP8J2qxNL4/ik8ibDq+Q9U=
+github.com/mattermost/mattermost-server v5.10.1+incompatible/go.mod h1:5L6MjAec+XXQwMIt791Ganu45GKsSiM+I0tLR9wUj8Y=
 github.com/mattermost/viper v0.0.0-20181112161711-f99c30686b86 h1:7/e3ksOHdNIdjBwKeM6acMWNkdLjwPM9MllJeLdfheY=
 github.com/mattermost/viper v0.0.0-20181112161711-f99c30686b86/go.mod h1:KAa5zVT6NsZa4/CLpDkT/A0LbI+lVEWMDNSjSHfgFO8=
 github.com/mitchellh/go-testing-interface v1.0.0 h1:fzU/JVNcaqHQEcVFAKeR41fkiLdIPrefOvVG1VZ96U0=
@@ -135,6 +139,8 @@ google.golang.org/genproto v0.0.0-20190404172233-64821d5d2107/go.mod h1:VzzqZJRn
 google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c=
 google.golang.org/grpc v1.20.0 h1:DlsSIrgEBuZAUFJcta2B5i/lzeHHbnfkNFAfFXLVFYQ=
 google.golang.org/grpc v1.20.0/go.mod h1:chYK+tFQF0nDUGJgXMSgLCQk3phJEuONr2DCgLDdAQM=
+gopkg.in/asn1-ber.v1 v1.0.0-20181015200546-f715ec2f112d h1:TxyelI5cVkbREznMhfzycHdkp5cLA7DpE+GKjSslYhM=
+gopkg.in/asn1-ber.v1 v1.0.0-20181015200546-f715ec2f112d/go.mod h1:cuepJuh7vyXfUyUwEgHQXw849cJrilpS5NeIjOWESAw=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/natefinch/lumberjack.v2 v2.0.0-20170531160350-a96e63847dc3 h1:AFxeG48hTWHhDTQDk/m2gorfVHUEa9vo3tp3D7TzwjI=

server/atlassian_connect.go

@@ -52,7 +52,7 @@ func httpACInstalled(p *Plugin, w http.ResponseWriter, r *http.Request) (int, er
 
 	// Only allow this operation once, a JIRA instance must already exist
 	// for asc.BaseURL but its EventType would be empty.
-	ji, err := p.LoadJIRAInstance(asc.BaseURL)
+	ji, err := p.instanceStore.LoadJIRAInstance(asc.BaseURL)
 	if err != nil {
 		return http.StatusInternalServerError,
 			errors.WithMessage(err, "failed to load instance "+asc.BaseURL)
@@ -72,7 +72,7 @@ func httpACInstalled(p *Plugin, w http.ResponseWriter, r *http.Request) (int, er
 
 	// Create a permanent instance record, also store it as current
 	jiraInstance := NewJIRACloudInstance(p, asc.BaseURL, true, string(body), &asc)
-	err = p.StoreJIRAInstance(jiraInstance)
+	err = p.instanceStore.StoreJIRAInstance(jiraInstance)
 	if err != nil {
 		return http.StatusInternalServerError, err
 	}

server/auth_token.go

@@ -31,7 +31,7 @@ func (p *Plugin) NewEncodedAuthToken(mattermostUserID, secret string) (returnTok
 		returnErr = errors.WithMessage(returnErr, "failed to create auth token")
 	}()
 
-	encryptSecret, err := p.EnsureAuthTokenEncryptSecret()
+	encryptSecret, err := p.secretsStore.EnsureAuthTokenEncryptSecret()
 	if err != nil {
 		return "", err
 	}
@@ -65,7 +65,7 @@ func (p *Plugin) ParseAuthToken(encoded string) (mattermostUserID, tokenSecret s
 
 	t := AuthToken{}
 	err := func() error {
-		encryptSecret, err := p.EnsureAuthTokenEncryptSecret()
+		encryptSecret, err := p.secretsStore.EnsureAuthTokenEncryptSecret()
 		if err != nil {
 			return err
 		}

server/command.go

@@ -105,13 +105,13 @@ func executeSettings(p *Plugin, c *plugin.Context, header *model.CommandArgs, ar
 		return help()
 	}
 
-	ji, err := p.LoadCurrentJIRAInstance()
+	ji, err := p.currentInstanceStore.LoadCurrentJIRAInstance()
 	if err != nil {
 		return responsef("Failed to load current Jira instance: %v. Please contact your system administrator.", err)
 	}
 
 	mattermostUserId := header.UserId
-	jiraUser, err := p.LoadJIRAUser(ji, mattermostUserId)
+	jiraUser, err := p.userStore.LoadJIRAUser(ji, mattermostUserId)
 	if err != nil {
 		return responsef("Your username is not connected to Jira. Please type `jira connect`. %v", err)
 	}
@@ -136,15 +136,15 @@ func executeList(p *Plugin, c *plugin.Context, header *model.CommandArgs, args .
 		return help()
 	}
 
-	known, err := p.LoadKnownJIRAInstances()
+	known, err := p.instanceStore.LoadKnownJIRAInstances()
 	if err != nil {
 		return responsef("Failed to load known Jira instances: %v", err)
 	}
 	if len(known) == 0 {
 		return responsef("(none installed)\n")
 	}
 
-	current, err := p.LoadCurrentJIRAInstance()
+	current, err := p.currentInstanceStore.LoadCurrentJIRAInstance()
 	if err != nil {
 		return responsef("Failed to load current Jira instance: %v", err)
 	}
@@ -156,7 +156,7 @@ func executeList(p *Plugin, c *plugin.Context, header *model.CommandArgs, args .
 	sort.Strings(keys)
 	text := "Known Jira instances (selected instance is **bold**)\n\n| |URL|Type|\n|--|--|--|\n"
 	for i, key := range keys {
-		ji, err := p.LoadJIRAInstance(key)
+		ji, err := p.instanceStore.LoadJIRAInstance(key)
 		if err != nil {
 			text += fmt.Sprintf("|%v|%s|error: %v|\n", i+1, key, err)
 			continue
@@ -205,7 +205,7 @@ func executeInstallCloud(p *Plugin, c *plugin.Context, header *model.CommandArgs
 
 	// Create an "uninitialized" instance of Jira Cloud that will
 	// receive the /installed callback
-	err = p.CreateInactiveCloudInstance(jiraURL)
+	err = p.instanceStore.CreateInactiveCloudInstance(jiraURL)
 	if err != nil {
 		return responsef(err.Error())
 	}
@@ -264,7 +264,7 @@ func executeInstallServer(p *Plugin, c *plugin.Context, header *model.CommandArg
 If you see an option to create a Jira issue, you're all set! If not, refer to our [documentation](https://about.mattermost.com/default-jira-plugin) for troubleshooting help.
 `
 	ji := NewJIRAServerInstance(p, jiraURL)
-	err = p.StoreJIRAInstance(ji)
+	err = p.instanceStore.StoreJIRAInstance(ji)
 	if err != nil {
 		return responsef(err.Error())
 	}
@@ -295,7 +295,7 @@ func executeUninstallCloud(p *Plugin, c *plugin.Context, header *model.CommandAr
 	}
 	jiraURL := args[0]
 
-	ji, err := p.LoadCurrentJIRAInstance()
+	ji, err := p.currentInstanceStore.LoadCurrentJIRAInstance()
 	if err != nil {
 		return responsef("No current Jira instance to uninstall")
 	}
@@ -309,7 +309,7 @@ func executeUninstallCloud(p *Plugin, c *plugin.Context, header *model.CommandAr
 		return responsef("You have entered an incorrect URL. The current Jira instance URL is: `" + jci.GetURL() + "`. Please enter the URL correctly to confirm the uninstall command.")
 	}
 
-	err = p.DeleteJiraInstance(jci.GetURL())
+	err = p.instanceStore.DeleteJiraInstance(jci.GetURL())
 	if err != nil {
 		return responsef("Failed to delete Jira instance " + ji.GetURL())
 	}
@@ -341,7 +341,7 @@ func executeUninstallServer(p *Plugin, c *plugin.Context, header *model.CommandA
 	}
 	jiraURL := args[0]
 
-	ji, err := p.LoadCurrentJIRAInstance()
+	ji, err := p.currentInstanceStore.LoadCurrentJIRAInstance()
 	if err != nil {
 		return responsef("No current Jira instance to uninstall")
 	}
@@ -355,7 +355,7 @@ func executeUninstallServer(p *Plugin, c *plugin.Context, header *model.CommandA
 		return responsef("You have entered an incorrect URL. The current Jira instance URL is: `" + jsi.GetURL() + "`. Please enter the URL correctly to confirm the uninstall command.")
 	}
 
-	err = p.DeleteJiraInstance(jsi.GetURL())
+	err = p.instanceStore.DeleteJiraInstance(jsi.GetURL())
 	if err != nil {
 		return responsef("Failed to delete Jira instance " + ji.GetURL())
 	}

server/http.go

@@ -58,11 +58,11 @@ func handleHTTPRequest(p *Plugin, w http.ResponseWriter, r *http.Request) (int,
 	switch r.URL.Path {
 	// Issue APIs
 	case routeAPICreateIssue:
-		return withInstance(p, w, r, httpAPICreateIssue)
+		return withInstance(p.currentInstanceStore, w, r, httpAPICreateIssue)
 	case routeAPIGetCreateIssueMetadata:
-		return withInstance(p, w, r, httpAPIGetCreateIssueMetadata)
+		return withInstance(p.currentInstanceStore, w, r, httpAPIGetCreateIssueMetadata)
 	case routeAPIAttachCommentToIssue:
-		return withInstance(p, w, r, httpAPIAttachCommentToIssue)
+		return withInstance(p.currentInstanceStore, w, r, httpAPIAttachCommentToIssue)
 
 	// User APIs
 	case routeAPIUserInfo:
@@ -96,9 +96,9 @@ func handleHTTPRequest(p *Plugin, w http.ResponseWriter, r *http.Request) (int,
 
 	// User connect/disconnect links
 	case routeUserConnect:
-		return withInstance(p, w, r, httpUserConnect)
+		return withInstance(p.currentInstanceStore, w, r, httpUserConnect)
 	case routeUserDisconnect:
-		return withInstance(p, w, r, httpUserDisconnect)
+		return withInstance(p.currentInstanceStore, w, r, httpUserDisconnect)
 
 	// Firehose webhook setup for channel subscriptions
 	case routeAPISubscribeWebhook:

server/instance.go

@@ -73,8 +73,8 @@ func (ji *JIRAInstance) Init(p *Plugin) {
 
 type withInstanceFunc func(ji Instance, w http.ResponseWriter, r *http.Request) (int, error)
 
-func withInstance(p *Plugin, w http.ResponseWriter, r *http.Request, f withInstanceFunc) (int, error) {
-	ji, err := p.LoadCurrentJIRAInstance()
+func withInstance(store CurrentInstanceStore, w http.ResponseWriter, r *http.Request, f withInstanceFunc) (int, error) {
+	ji, err := store.LoadCurrentJIRAInstance()
 	if err != nil {
 		return http.StatusInternalServerError, err
 	}

server/instance_cloud.go

@@ -60,7 +60,7 @@ func NewJIRACloudInstance(p *Plugin, key string, installed bool, rawASC string,
 type withCloudInstanceFunc func(jci *jiraCloudInstance, w http.ResponseWriter, r *http.Request) (int, error)
 
 func withCloudInstance(p *Plugin, w http.ResponseWriter, r *http.Request, f withCloudInstanceFunc) (int, error) {
-	return withInstance(p, w, r, func(ji Instance, w http.ResponseWriter, r *http.Request) (int, error) {
+	return withInstance(p.currentInstanceStore, w, r, func(ji Instance, w http.ResponseWriter, r *http.Request) (int, error) {
 		jci, ok := ji.(*jiraCloudInstance)
 		if !ok {
 			return http.StatusBadRequest, errors.New("Must be a JIRA Cloud instance, is " + ji.GetType())
@@ -96,7 +96,7 @@ func (jci jiraCloudInstance) GetUserConnectURL(mattermostUserId string) (string,
 	}
 	secretKey := fmt.Sprintf("%x", sha256.Sum256(secret))
 	secretValue := "true"
-	err = jci.Plugin.StoreOneTimeSecret(secretKey, secretValue)
+	err = jci.Plugin.otsStore.StoreOneTimeSecret(secretKey, secretValue)
 	if err != nil {
 		return "", err
 	}

server/instance_server.go

@@ -39,7 +39,7 @@ func (jsi jiraServerInstance) GetURL() string {
 type withServerInstanceFunc func(jsi *jiraServerInstance, w http.ResponseWriter, r *http.Request) (int, error)
 
 func withServerInstance(p *Plugin, w http.ResponseWriter, r *http.Request, f withServerInstanceFunc) (int, error) {
-	return withInstance(p, w, r, func(ji Instance, w http.ResponseWriter, r *http.Request) (int, error) {
+	return withInstance(p.currentInstanceStore, w, r, func(ji Instance, w http.ResponseWriter, r *http.Request) (int, error) {
 		jsi, ok := ji.(*jiraServerInstance)
 		if !ok {
 			return http.StatusBadRequest, errors.New("Must be a Jira Server instance, is " + ji.GetType())
@@ -76,7 +76,7 @@ func (jsi jiraServerInstance) GetUserConnectURL(mattermostUserId string) (return
 		return "", err
 	}
 
-	err = jsi.Plugin.StoreOneTimeSecret(token, secret)
+	err = jsi.Plugin.otsStore.StoreOneTimeSecret(token, secret)
 	if err != nil {
 		return "", err
 	}
@@ -136,7 +136,7 @@ func (jsi *jiraServerInstance) GetOAuth1Config() (returnConfig *oauth1.Config, r
 		return oauth1Config, nil
 	}
 
-	rsaKey, err := jsi.EnsureRSAKey()
+	rsaKey, err := jsi.secretsStore.EnsureRSAKey()
 	if err != nil {
 		return nil, err
 	}

server/issue.go

@@ -41,7 +41,7 @@ func httpAPICreateIssue(ji Instance, w http.ResponseWriter, r *http.Request) (in
 		return http.StatusUnauthorized, errors.New("not authorized")
 	}
 
-	jiraUser, err := ji.GetPlugin().LoadJIRAUser(ji, mattermostUserId)
+	jiraUser, err := ji.GetPlugin().userStore.LoadJIRAUser(ji, mattermostUserId)
 	if err != nil {
 		return http.StatusInternalServerError, err
 	}
@@ -173,7 +173,7 @@ func httpAPIGetCreateIssueMetadata(ji Instance, w http.ResponseWriter, r *http.R
 		return http.StatusUnauthorized, errors.New("not authorized")
 	}
 
-	jiraUser, err := ji.GetPlugin().LoadJIRAUser(ji, mattermostUserId)
+	jiraUser, err := ji.GetPlugin().userStore.LoadJIRAUser(ji, mattermostUserId)
 	if err != nil {
 		return http.StatusInternalServerError, err
 	}
@@ -235,7 +235,7 @@ func httpAPIAttachCommentToIssue(ji Instance, w http.ResponseWriter, r *http.Req
 		return http.StatusUnauthorized, errors.New("not authorized")
 	}
 
-	jiraUser, err := ji.GetPlugin().LoadJIRAUser(ji, mattermostUserId)
+	jiraUser, err := ji.GetPlugin().userStore.LoadJIRAUser(ji, mattermostUserId)
 	if err != nil {
 		return http.StatusInternalServerError, err
 	}
@@ -339,12 +339,12 @@ func getPermaLink(ji Instance, postId string, post *model.Post) (string, error)
 }
 
 func (p *Plugin) transitionJiraIssue(mmUserId, issueKey, toState string) (string, error) {
-	ji, err := p.LoadCurrentJIRAInstance()
+	ji, err := p.currentInstanceStore.LoadCurrentJIRAInstance()
 	if err != nil {
 		return "", err
 	}
 
-	jiraUser, err := ji.GetPlugin().LoadJIRAUser(ji, mmUserId)
+	jiraUser, err := ji.GetPlugin().userStore.LoadJIRAUser(ji, mmUserId)
 	if err != nil {
 		return "", err
 	}