From 6656d9d4a980ec2e741978998a14b55154c4129c Mon Sep 17 00:00:00 2001 From: Lev <1187448+levb@users.noreply.github.com> Date: Fri, 14 Jun 2019 07:51:35 -0700 Subject: [PATCH] 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 --- Makefile | 2 +- server/command.go | 2 +- server/instance_cloud.go | 18 +++++++++++------- server/instance_server.go | 2 +- server/issue.go | 2 +- server/kv.go | 25 ++++++------------------- server/user.go | 7 ++++--- server/user_cloud.go | 11 ++++++++--- server/user_server.go | 1 + 9 files changed, 34 insertions(+), 36 deletions(-) diff --git a/Makefile b/Makefile index 99a61119e..426897d29 100644 --- 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 index 226550cd6..83cccabd0 100644 --- 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 index 4816c81e5..7b4d22b23 100644 --- 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 index 924729175..5f3db189d 100644 --- 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 index 93ddee7e4..20077775e 100644 --- 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 index 50ec4cfcb..601e147fd 100644 --- a/server/kv.go +++ b/server/kv.go @@ -320,7 +320,7 @@ 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) @@ -328,14 +328,14 @@ func (p *Plugin) StoreUserInfo(ji Instance, mattermostUserId string, jiraUser JI 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 index 6e348e138..81e3c64d6 100644 --- 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 index 5f1f9c385..a3d6f9801 100644 --- 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 index ffed12784..8637b8406 100644 --- 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 {