MM-16364 Use AccountID as applicable rather than Name (#166)

* MM-16364 Use AccoutID as applicable rather than Name - Tried to reduce the use of `.Name` to thee minimum outside of Jira Server-specific code. - removed rather than change LoadMattermostUserId() since it is not used in this version - Also patched `make deploy` to the current Token header format * Fixup
mattermost · Jun 14, 2019 · 6656d9d · 6656d9d
1 parent d64b748
commit 6656d9d
Show file tree

Hide file tree

Showing 9 changed files with 34 additions and 36 deletions.
diff --git a/Makefile b/Makefile
@@ -123,7 +123,7 @@ deploy: dist
 ## or copying the files directly to a sibling mattermost-server directory.
 ifneq ($(and $(MM_SERVICESETTINGS_SITEURL),$(MM_ADMIN_USERNAME),$(MM_ADMIN_PASSWORD),$(CURL)),)
 	@echo "Installing plugin via API"
-	$(eval TOKEN := $(shell curl -i -X POST $(MM_SERVICESETTINGS_SITEURL)/api/v4/users/login -d '{"login_id": "$(MM_ADMIN_USERNAME)", "password": "$(MM_ADMIN_PASSWORD)"}' | grep -o MMAUTHTOKEN=[0-9a-z]\* | cut -f2 -d'=' 2> /dev/null))
+	$(eval TOKEN := $(shell curl -i -X POST $(MM_SERVICESETTINGS_SITEURL)/api/v4/users/login -d '{"login_id": "$(MM_ADMIN_USERNAME)", "password": "$(MM_ADMIN_PASSWORD)"}' | grep -o "Token: [0-9a-z]*" | cut -f2 -d' ' 2> /dev/null))
 	@curl -s -H "Authorization: Bearer $(TOKEN)" -X POST $(MM_SERVICESETTINGS_SITEURL)/api/v4/plugins -F "plugin=@dist/$(BUNDLE_NAME)" -F "force=true" > /dev/null && \
 		curl -s -H "Authorization: Bearer $(TOKEN)" -X POST $(MM_SERVICESETTINGS_SITEURL)/api/v4/plugins/$(PLUGIN_ID)/enable > /dev/null && \
 		echo "OK." || echo "Sorry, something went wrong."

diff --git a/server/command.go b/server/command.go
@@ -101,7 +101,7 @@ func executeDisconnect(p *Plugin, c *plugin.Context, header *model.CommandArgs,
 		return responsef(p, "Could not complete the **disconnection** request. Error: %v", err)
 	}
 
-	return responsef(p, "You have successfully disconnected your Jira account (**%s**).", jiraUser.Name)
+	return responsef(p, "You have successfully disconnected your Jira account (**%s**).", jiraUser.DisplayName)
 }
 
 func executeList(p *Plugin, c *plugin.Context, header *model.CommandArgs, args ...string) *model.CommandResponse {

diff --git a/server/instance_cloud.go b/server/instance_cloud.go
@@ -14,6 +14,7 @@ import (
 	"github.com/dgrijalva/jwt-go"
 	"github.com/pkg/errors"
 	ajwt "github.com/rbriski/atlassian-jwt"
+	"golang.org/x/oauth2"
 	oauth2_jira "golang.org/x/oauth2/jira"
 )
 
@@ -123,7 +124,7 @@ func (jci jiraCloudInstance) GetJIRAClient(jiraUser JIRAUser) (*jira.Client, err
 	//TODO decide if we ever need this as the default client
 	// client, err = jci.getJIRAClientForServer()
 	if err != nil {
-		return nil, errors.WithMessage(err, "failed to get Jira client for user "+jiraUser.Name)
+		return nil, errors.WithMessagef(err, "failed to get Jira client for %q", jiraUser.DisplayName)
 	}
 
 	return client, nil
@@ -133,14 +134,17 @@ func (jci jiraCloudInstance) GetJIRAClient(jiraUser JIRAUser) (*jira.Client, err
 func (jci jiraCloudInstance) getJIRAClientForUser(jiraUser JIRAUser) (*jira.Client, *http.Client, error) {
 	oauth2Conf := oauth2_jira.Config{
 		BaseURL: jci.GetURL(),
-		Subject: jiraUser.Name,
+		Subject: jiraUser.AccountID,
+		Config: oauth2.Config{
+			ClientID:     jci.AtlassianSecurityContext.OAuthClientId,
+			ClientSecret: jci.AtlassianSecurityContext.SharedSecret,
+			Endpoint: oauth2.Endpoint{
+				AuthURL:  "https://auth.atlassian.io",
+				TokenURL: "https://auth.atlassian.io/oauth2/token",
+			},
+		},
 	}
 
-	oauth2Conf.Config.ClientID = jci.AtlassianSecurityContext.OAuthClientId
-	oauth2Conf.Config.ClientSecret = jci.AtlassianSecurityContext.SharedSecret
-	oauth2Conf.Config.Endpoint.AuthURL = "https://auth.atlassian.io"
-	oauth2Conf.Config.Endpoint.TokenURL = "https://auth.atlassian.io/oauth2/token"
-
 	httpClient := oauth2Conf.Client(context.Background())
 
 	jiraClient, err := jira.NewClient(httpClient, oauth2Conf.BaseURL)

diff --git a/server/instance_server.go b/server/instance_server.go
@@ -94,7 +94,7 @@ func (jsi jiraServerInstance) GetJIRAClient(jiraUser JIRAUser) (returnClient *ji
 		if returnErr == nil {
 			return
 		}
-		returnErr = errors.WithMessage(returnErr, "failed to get a Jira client for "+jiraUser.Name)
+		returnErr = errors.WithMessagef(returnErr, "failed to get a Jira client for %q", jiraUser.DisplayName)
 	}()
 
 	if jiraUser.Oauth1AccessToken == "" || jiraUser.Oauth1AccessSecret == "" {

diff --git a/server/issue.go b/server/issue.go
@@ -405,7 +405,7 @@ func httpAPIAttachCommentToIssue(ji Instance, w http.ResponseWriter, r *http.Req
 
 	permalink := getPermaLink(ji, attach.PostId, attach.CurrentTeam)
 
-	permalinkMessage := fmt.Sprintf("*@%s attached a* [message|%s] *from @%s*\n", jiraUser.User.Name, permalink, commentUser.Username)
+	permalinkMessage := fmt.Sprintf("*@%s attached a* [message|%s] *from %s*\n", jiraUser.User.DisplayName, permalink, commentUser.Username)
 
 	var jiraComment jira.Comment
 	jiraComment.Body = permalinkMessage

diff --git a/server/kv.go b/server/kv.go
@@ -320,22 +320,22 @@ func (p *Plugin) StoreUserInfo(ji Instance, mattermostUserId string, jiraUser JI
 			return
 		}
 		returnErr = errors.WithMessage(returnErr,
-			fmt.Sprintf("failed to store user, mattermostUserId:%s, Jira user:%s", mattermostUserId, jiraUser.Name))
+			fmt.Sprintf("failed to store user, mattermostUserId:%s, Jira user:%q", mattermostUserId, jiraUser.DisplayName))
 	}()
 
 	err := p.kvSet(keyWithInstance(ji, mattermostUserId), jiraUser)
 	if err != nil {
 		return err
 	}
 
-	err = p.kvSet(keyWithInstance(ji, jiraUser.Name), mattermostUserId)
+	err = p.kvSet(keyWithInstance(ji, jiraUser.UserKey), mattermostUserId)
 	if err != nil {
 		return err
 	}
 
 	p.debugf("Stored: Jira user, keys:\n\t%s (%s): %+v\n\t%s (%s): %s",
 		keyWithInstance(ji, mattermostUserId), mattermostUserId, jiraUser,
-		keyWithInstance(ji, jiraUser.Name), jiraUser.Name, mattermostUserId)
+		keyWithInstance(ji, jiraUser.UserKey), jiraUser.DisplayName, mattermostUserId)
 
 	return nil
 }
@@ -355,19 +355,6 @@ func (p *Plugin) LoadJIRAUser(ji Instance, mattermostUserId string) (JIRAUser, e
 	return jiraUser, nil
 }
 
-func (p *Plugin) LoadMattermostUserId(ji Instance, jiraUserName string) (string, error) {
-	mattermostUserId := ""
-	err := p.kvGet(keyWithInstance(ji, jiraUserName), &mattermostUserId)
-	if err != nil {
-		return "", errors.WithMessage(err,
-			"failed to load Mattermost user ID for Jira user: "+jiraUserName)
-	}
-	if len(mattermostUserId) == 0 {
-		return "", ErrUserNotFound
-	}
-	return mattermostUserId, nil
-}
-
 func (p *Plugin) DeleteUserInfo(ji Instance, mattermostUserId string) (returnErr error) {
 	defer func() {
 		if returnErr == nil {
@@ -387,14 +374,14 @@ func (p *Plugin) DeleteUserInfo(ji Instance, mattermostUserId string) (returnErr
 		return appErr
 	}
 
-	appErr = p.API.KVDelete(keyWithInstance(ji, jiraUser.Name))
+	appErr = p.API.KVDelete(keyWithInstance(ji, jiraUser.UserKey))
 	if appErr != nil {
 		return appErr
 	}
 
-	p.debugf("Deleted: user, keys: %s(%s), %s(%s)",
+	p.debugf("Deleted: user, keys: %s (%s), %q (%s)",
 		mattermostUserId, keyWithInstance(ji, mattermostUserId),
-		jiraUser.Name, keyWithInstance(ji, jiraUser.Name))
+		jiraUser.DisplayName, keyWithInstance(ji, jiraUser.UserKey))
 	return nil
 }
 

diff --git a/server/user.go b/server/user.go
@@ -20,6 +20,8 @@ const (
 
 type JIRAUser struct {
 	jira.User
+	// TODO replace with jira.User.Key if it can be relied upon
+	UserKey            string
 	Oauth1AccessToken  string `json:",omitempty"`
 	Oauth1AccessSecret string `json:",omitempty"`
 }
@@ -158,9 +160,8 @@ func (p *Plugin) StoreUserInfoNotify(ji Instance, mattermostUserId string, jiraU
 	p.API.PublishWebSocketEvent(
 		WS_EVENT_CONNECT,
 		map[string]interface{}{
-			"is_connected":  true,
-			"jira_username": jiraUser.Name,
-			"jira_url":      ji.GetURL(),
+			"is_connected": true,
+			"jira_url":     ji.GetURL(),
 		},
 		&model.WebsocketBroadcast{UserId: mattermostUserId},
 	)

diff --git a/server/user_cloud.go b/server/user_cloud.go
@@ -60,15 +60,20 @@ func httpACUserInteractive(jci *jiraCloudInstance, w http.ResponseWriter, r *htt
 	if !ok {
 		return http.StatusBadRequest, errors.New("invalid JWT: no user data")
 	}
+
+	accountId, _ := user["accountId"].(string)
+	displayName, _ := user["displayName"].(string)
 	userKey, _ := user["userKey"].(string)
 	username, _ := user["username"].(string)
-	displayName, _ := user["displayName"].(string)
 
 	mmToken := r.Form.Get(argMMToken)
 	uinfo := JIRAUser{
+		UserKey: accountId,
 		User: jira.User{
-			Key:  userKey,
-			Name: username,
+			AccountID:   accountId,
+			DisplayName: displayName,
+			Key:         userKey,
+			Name:        username,
 		},
 	}
 	mattermostUserId := r.Header.Get("Mattermost-User-ID")

diff --git a/server/user_server.go b/server/user_server.go
@@ -75,6 +75,7 @@ func httpOAuth1Complete(jsi *jiraServerInstance, w http.ResponseWriter, r *http.
 		return http.StatusInternalServerError, err
 	}
 	jiraUser.User = *juser
+	jiraUser.UserKey = juser.Name
 
 	err = jsi.Plugin.StoreUserInfoNotify(jsi, mattermostUserId, jiraUser)
 	if err != nil {