Implemented few additional review comments

mattermost · Apr 27, 2023 · 326992f · 326992f
1 parent b64b4d5
commit 326992f
Show file tree

Hide file tree

Showing 7 changed files with 18 additions and 34 deletions.
diff --git a/assets/templates/oauth2/complete.html b/assets/templates/oauth2/complete.html
@@ -1,6 +1,10 @@
 <!DOCTYPE html>
 <html lang="en">
 	<head>
+		<script>
+			window.open('','_parent','');
+			window.close();
+		</script>
 		<style>
 			body {
 				color: rgb(23, 43, 77);
@@ -75,4 +79,3 @@ <h3>
 		</div>
 	</body>
 </html>
-
diff --git a/server/command.go b/server/command.go
@@ -79,7 +79,7 @@ const commonHelpText = "\n" +
 
 const sysAdminHelpText = "\n###### For System Administrators:\n" +
 	"Install Jira instances:\n" +
-	"* `/jira instance install cloud [jiraURL]` - Deprecated: Connect Mattermost to a Jira Cloud instance located at <jiraURL>\n" +
+	"* `/jira instance install cloud [jiraURL]` - Connect Mattermost to a Jira Cloud instance located at <jiraURL>. (Deprecated. Please use `cloud-oauth` instead.)\n" +
 	"* `/jira instance install cloud-oauth [jiraURL]` - Connect Mattermost to a Jira Cloud instance using OAuth 2.0 located at <jiraURL>\n" +
 	"* `/jira instance install server [jiraURL]` - Connect Mattermost to a Jira Server or Data Center instance located at <jiraURL>\n" +
 	"Uninstall Jira instances:\n" +
@@ -171,7 +171,7 @@ func createInstanceCommand(optInstance bool) *model.AutocompleteData {
 
 	jiraTypes := []model.AutocompleteListItem{
 		{HelpText: "Jira Server or Datacenter", Item: "server"},
-		{HelpText: "(Deprecated) Jira Cloud (atlassian.net)", Item: "cloud"},
+		{HelpText: "Jira Cloud (atlassian.net) (Deprecated. Please use cloud-oauth instead.)", Item: "cloud"},
 		{HelpText: "Jira Cloud OAuth 2.0 (atlassian.net)", Item: "cloud-oauth"},
 	}
 
@@ -808,7 +808,7 @@ func executeInstanceInstallCloudOAuth(p *Plugin, c *plugin.Context, header *mode
 		return p.responsef(header, "%v", err)
 	}
 	if !authorized {
-		return p.responsef(header, "`/jira install` can only be run by a system administrator.")
+		return p.responsef(header, "`/jira install` can only be run by a Mattermost system administrator.")
 	}
 	if len(args) != 1 {
 		return p.help(header)

diff --git a/server/instance_cloud_oauth.go b/server/instance_cloud_oauth.go
@@ -88,24 +88,14 @@ func (ci *cloudOAuthInstance) getClientForConnection(connection *Connection) (*j
 	client := oauth2.NewClient(ctx, tokenSource)
 
 	// Get a new token if Access Token has expired
-	if ci.Plugin.IsAccessTokenExpired(connection.OAuth2Token) {
-		ci.client.Log.Debug("AccessToken is about to expire (or) has expired. Refreshing the accessToken.")
-
-		// Set the AccessToken to empty in order for refresh to happen forcefully
-		token := connection.OAuth2Token
-		token.AccessToken = ""
-		tokenSource := oauth2Conf.TokenSource(ctx, token)
-
-		// Refresh the Token with its own mutex.
-		newToken, err := tokenSource.Token()
-		if err != nil {
-			ci.client.Log.Warn("Error while refreshing the Jira Access Token", "error", err)
-			return nil, nil, err
-		}
+	currentToken := connection.OAuth2Token
+	updatedToken, err := tokenSource.Token()
+	if err != nil {
+		return nil, nil, errors.Wrap(err, "error getting token from token source")
+	}
 
-		connection.OAuth2Token = newToken
-		tokenSource = oauth2Conf.TokenSource(ctx, newToken)
-		client = oauth2.NewClient(ctx, tokenSource)
+	if updatedToken.RefreshToken != currentToken.RefreshToken {
+		connection.OAuth2Token = updatedToken
 
 		// Store this new access token & refresh token to get a new access token in future when it has expired
 		err = ci.Plugin.userStore.StoreConnection(ci.Common().InstanceID, connection.MattermostUserID, connection)

diff --git a/server/issue_parser.go b/server/issue_parser.go
@@ -21,7 +21,7 @@ func parseJiraLinksToMarkdown(text string) string {
 }
 
 func mdKeySummaryLink(issue *jira.Issue, instance Instance) string {
-	return fmt.Sprintf("[%s](%s%s)", issue.Key+": "+issue.Fields.Summary+" ("+issue.Fields.Status.Name+")", instance.GetJiraBaseURL(), "/browse/"+issue.Key)
+	return fmt.Sprintf("[%s: %s (%s)](%s%s)", issue.Key, issue.Fields.Summary, issue.Fields.Status.Name, instance.GetJiraBaseURL(), "/browse/"+issue.Key)
 }
 
 func reporterSummary(issue *jira.Issue) string {

diff --git a/server/kv.go b/server/kv.go
@@ -138,6 +138,7 @@ func (store store) StoreConnection(instanceID, mattermostUserID types.ID, connec
 	}()
 
 	connection.PluginVersion = manifest.Version
+	connection.MattermostUserID = mattermostUserID
 
 	err := store.set(keyWithInstanceID(instanceID, mattermostUserID), connection)
 	if err != nil {

diff --git a/server/setup_flow.go b/server/setup_flow.go
@@ -101,7 +101,6 @@ func (p *Plugin) NewOAuth2Flow() *flow.Flow {
 			p.stepCancel(),
 			p.stepDone(),
 		).
-		// WithDebugLog().
 		InitHTTP(p.router)
 }
 

diff --git a/server/user_cloud_oauth.go b/server/user_cloud_oauth.go
@@ -8,11 +8,9 @@ import (
 	"net/http"
 	"path"
 	"strings"
-	"time"
 
 	"github.com/mattermost/mattermost-server/v6/model"
 	"github.com/pkg/errors"
-	"golang.org/x/oauth2"
 
 	"github.com/mattermost/mattermost-plugin-jira/server/utils/types"
 )
@@ -54,7 +52,7 @@ func (p *Plugin) httpOAuth2Complete(w http.ResponseWriter, r *http.Request, inst
 		return http.StatusInternalServerError, err
 	}
 
-	connection, err := p.GenerateOAuthToken(mattermostUserID, instanceID, code)
+	connection, err := p.GenerateInitialOAuthToken(mattermostUserID, instanceID, code)
 	if err != nil {
 		return http.StatusInternalServerError, err
 	}
@@ -90,7 +88,7 @@ func (p *Plugin) httpOAuth2Complete(w http.ResponseWriter, r *http.Request, inst
 	})
 }
 
-func (p *Plugin) GenerateOAuthToken(mattermostUserID string, instanceID types.ID, code string) (*Connection, error) {
+func (p *Plugin) GenerateInitialOAuthToken(mattermostUserID string, instanceID types.ID, code string) (*Connection, error) {
 	instance, err := p.instanceStore.LoadInstance(instanceID)
 	if err != nil {
 		return nil, err
@@ -116,10 +114,3 @@ func (p *Plugin) GenerateOAuthToken(mattermostUserID string, instanceID types.ID
 	connection.OAuth2Token = token
 	return connection, nil
 }
-
-// checks if a user's access token is expired
-func (p *Plugin) IsAccessTokenExpired(token *oauth2.Token) bool {
-	// Consider some buffer for comparing expiry time
-	localExpiryTime := time.Unix(token.Expiry.Unix(), 0).Local()
-	return time.Until(localExpiryTime) <= time.Minute*TokenExpiryTimeBufferInMinutes
-}