codreview #3: two credential types, more fixes

* Support both token and session credential types
* use getTimeDervedID in export.go
* keyOrigin -> core.KeyOrigin
* fix one indentation
* remove project key from operation metadata
* fix missing credentials codepath if not using sidecar

4 files changed, 126 insertions, 59 deletions

diff --git a/bridge/jira/client.go b/bridge/jira/client.go
index 6dd25ccb..adaad94d 100644
--- a/bridge/jira/client.go
+++ b/bridge/jira/client.go
@@ -3,6 +3,7 @@ package jira
 import (
 	"bytes"
 	"context"
+	"encoding/base64"
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
@@ -305,15 +306,27 @@ type ServerInfo struct {
 // "Content-Type=application/json" header
 type ClientTransport struct {
 	underlyingTransport http.RoundTripper
+	basicAuthString     string
 }
 
 // RoundTrip overrides the default by adding the content-type header
 func (self *ClientTransport) RoundTrip(
 	req *http.Request) (*http.Response, error) {
 	req.Header.Add("Content-Type", "application/json")
+	if self.basicAuthString != "" {
+		req.Header.Add("Authorization",
+			fmt.Sprintf("Basic %s", self.basicAuthString))
+	}
+
 	return self.underlyingTransport.RoundTrip(req)
 }
 
+func (self *ClientTransport) SetCredentials(
+	username string, token string) {
+	credString := fmt.Sprintf("%s:%s", username, token)
+	self.basicAuthString = base64.StdEncoding.EncodeToString([]byte(credString))
+}
+
 // Client Thin wrapper around the http.Client providing jira-specific methods
 // for APIendpoints
 type Client struct {
@@ -336,12 +349,26 @@ func NewClient(serverURL string, ctx context.Context) *Client {
 
 // Login POST credentials to the /session endpoing and get a session cookie
 func (client *Client) Login(conf core.Configuration) error {
+	credType := conf[keyCredentialsType]
+
 	if conf[keyCredentialsFile] != "" {
 		content, err := ioutil.ReadFile(conf[keyCredentialsFile])
 		if err != nil {
 			return err
 		}
-		return client.RefreshTokenRaw(content)
+
+		switch credType {
+		case "SESSION":
+			return client.RefreshSessionTokenRaw(content)
+		case "TOKEN":
+			var params SessionQuery
+			err := json.Unmarshal(content, &params)
+			if err != nil {
+				return err
+			}
+			return client.SetTokenCredentials(params.Username, params.Password)
+		}
+		return fmt.Errorf("Unexpected credType: %s", credType)
 	}
 
 	username := conf[keyUsername]
@@ -360,12 +387,18 @@ func (client *Client) Login(conf core.Configuration) error {
 		}
 	}
 
-	return client.RefreshToken(username, password)
+	switch credType {
+	case "SESSION":
+		return client.RefreshSessionToken(username, password)
+	case "TOKEN":
+		return client.SetTokenCredentials(username, password)
+	}
+	return fmt.Errorf("Unexpected credType: %s", credType)
 }
 
-// RefreshToken formulate the JSON request object from the user credentials
-// and POST it to the /session endpoing and get a session cookie
-func (client *Client) RefreshToken(username, password string) error {
+// RefreshSessionToken formulate the JSON request object from the user
+// credentials and POST it to the /session endpoing and get a session cookie
+func (client *Client) RefreshSessionToken(username, password string) error {
 	params := SessionQuery{
 		Username: username,
 		Password: password,
@@ -376,12 +409,24 @@ func (client *Client) RefreshToken(username, password string) error {
 		return err
 	}
 
-	return client.RefreshTokenRaw(data)
+	return client.RefreshSessionTokenRaw(data)
+}
+
+// SetTokenCredentials POST credentials to the /session endpoing and get a
+// session cookie
+func (client *Client) SetTokenCredentials(username, password string) error {
+	switch transport := client.Transport.(type) {
+	case *ClientTransport:
+		transport.SetCredentials(username, password)
+	default:
+		return fmt.Errorf("Invalid transport type")
+	}
+	return nil
 }
 
-// RefreshTokenRaw POST credentials to the /session endpoing and get a session
-// cookie
-func (client *Client) RefreshTokenRaw(credentialsJSON []byte) error {
+// RefreshSessionTokenRaw POST credentials to the /session endpoing and get a
+// session cookie
+func (client *Client) RefreshSessionTokenRaw(credentialsJSON []byte) error {
 	postURL := fmt.Sprintf("%s/rest/auth/1/session", client.serverURL)
 
 	req, err := http.NewRequest("POST", postURL, bytes.NewBuffer(credentialsJSON))
diff --git a/bridge/jira/config.go b/bridge/jira/config.go
index 41070228..e23ee845 100644
--- a/bridge/jira/config.go
+++ b/bridge/jira/config.go
@@ -21,6 +21,7 @@ const (
 	target             = "jira"
 	keyServer          = "server"
 	keyProject         = "project"
+	keyCredentialsType = "credentials-type"
 	keyCredentialsFile = "credentials-file"
 	keyUsername        = "username"
 	keyPassword        = "password"
@@ -38,6 +39,30 @@ see the notes at:
 https://github.com/MichaelMure/git-bug/blob/master/doc/jira_bridge.md
 `
 
+const credTypeText = `
+JIRA has recently altered it's authentication strategies. Servers deployed
+prior to October 1st 2019 must use "SESSION" authentication, whereby the REST
+client logs in with an actual username and password, is assigned a session, and
+passes the session cookie with each request. JIRA Cloud and servers deployed
+after October 1st 2019 must use "TOKEN" authentication. You must create a user
+API token and the client will provide this along with your username with each
+request.
+
+Which authentication mechanism should this bridge use?
+[1]: SESSION
+[2]: TOKEN
+`
+const credentialsText = `
+How would you like to store your JIRA login credentials?
+[1]: sidecar JSON file: Your credentials will be stored in a JSON sidecar next
+     to your git config. Note that it will contain your JIRA password in clear
+     text.
+[2]: git-config: Your credentials will be stored in the git config. Note that
+     it will contain your JIRA password in clear text.
+[3]: username in config, askpass: Your username will be stored in the git
+     config. We will ask you for your password each time you execute the bridge.
+`
+
 // Configure sets up the bridge configuration
 func (g *Jira) Configure(
 	repo repository.RepoCommon, params core.BridgeParams) (
@@ -78,7 +103,12 @@ func (g *Jira) Configure(
 		}
 	}
 
-	choice, err := promptCredentialOptions(serverURL)
+	credType, err := promptOptions(credTypeText, 1, 2)
+	if err != nil {
+		return nil, err
+	}
+
+	choice, err := promptOptions(credentialsText, 1, 3)
 	if err != nil {
 		return nil, err
 	}
@@ -106,25 +136,17 @@ func (g *Jira) Configure(
 		return nil, err
 	}
 
-	fmt.Printf("Attempting to login with credentials...\n")
-	client := NewClient(serverURL, nil)
-	err = client.RefreshTokenRaw(jsonData)
-	if err != nil {
-		return nil, err
-	}
-
-	// verify access to the project with credentials
-	_, err = client.GetProject(project)
-	if err != nil {
-		return nil, fmt.Errorf(
-			"Project %s doesn't exist on %s, or authentication credentials for (%s)"+
-				" are invalid",
-			project, serverURL, username)
-	}
-
 	conf[core.KeyTarget] = target
 	conf[keyServer] = serverURL
 	conf[keyProject] = project
+
+	switch credType {
+	case 1:
+		conf[keyCredentialsType] = "SESSION"
+	case 2:
+		conf[keyCredentialsType] = "TOKEN"
+	}
+
 	switch choice {
 	case 1:
 		conf[keyCredentialsFile] = credentialsFile
@@ -145,6 +167,23 @@ func (g *Jira) Configure(
 		return nil, err
 	}
 
+	fmt.Printf("Attempting to login with credentials...\n")
+	client := NewClient(serverURL, nil)
+	err = client.Login(conf)
+	if err != nil {
+		return nil, err
+	}
+
+	// verify access to the project with credentials
+	fmt.Printf("Checking project ...\n")
+	_, err = client.GetProject(project)
+	if err != nil {
+		return nil, fmt.Errorf(
+			"Project %s doesn't exist on %s, or authentication credentials for (%s)"+
+				" are invalid",
+			project, serverURL, username)
+	}
+
 	fmt.Print(moreConfigText)
 	return conf, nil
 }
@@ -164,19 +203,8 @@ func (*Jira) ValidateConfig(conf core.Configuration) error {
 	return nil
 }
 
-const credentialsText = `
-How would you like to store your JIRA login credentials?
-[1]: sidecar JSON file: Your credentials will be stored in a JSON sidecar next
-     to your git config. Note that it will contain your JIRA password in clear
-     text.
-[2]: git-config: Your credentials will be stored in the git config. Note that
-     it will contain your JIRA password in clear text.
-[3]: username in config, askpass: Your username will be stored in the git
-     config. We will ask you for your password each time you execute the bridge.
-`
-
-func promptCredentialOptions(serverURL string) (int, error) {
-	fmt.Print(credentialsText)
+func promptOptions(description string, minVal, maxVal int) (int, error) {
+	fmt.Print(description)
 	for {
 		fmt.Print("Select option: ")
 
@@ -189,10 +217,14 @@ func promptCredentialOptions(serverURL string) (int, error) {
 		line = strings.TrimRight(line, "\n")
 
 		index, err := strconv.Atoi(line)
-		if err != nil || (index != 1 && index != 2 && index != 3) {
+		if err != nil {
 			fmt.Println("invalid input")
 			continue
 		}
+		if index < minVal || index > maxVal {
+			fmt.Println("invalid choice")
+			continue
+		}
 
 		return index, nil
 	}
diff --git a/bridge/jira/export.go b/bridge/jira/export.go
index 8d8d326d..02ec5e14 100644
--- a/bridge/jira/export.go
+++ b/bridge/jira/export.go
@@ -171,7 +171,7 @@ func (self *jiraExporter) exportBug(
 	author := snapshot.Author
 
 	// skip bug if it was imported from some other bug system
-	origin, ok := snapshot.GetCreateMetadata(keyOrigin)
+	origin, ok := snapshot.GetCreateMetadata(core.KeyOrigin)
 	if ok && origin != target {
 		out <- core.NewExportNothing(
 			b.Id(), fmt.Sprintf("issue tagged with origin: %s", origin))
@@ -342,7 +342,7 @@ func (self *jiraExporter) exportBug(
 				// so we use the comment ID plus the timestamp of the update, as
 				// reported by JIRA. Note that this must be consistent with the importer
 				// during ensureComment()
-				id = fmt.Sprintf("%s-%d", comment.ID, comment.Updated.Unix())
+				id = getTimeDerivedID(comment.ID, comment.Updated)
 			}
 
 		case *bug.SetStatusOperation:
diff --git a/bridge/jira/import.go b/bridge/jira/import.go
index 96ef81ab..346aa6fd 100644
--- a/bridge/jira/import.go
+++ b/bridge/jira/import.go
@@ -17,7 +17,6 @@ import (
 )
 
 const (
-	keyOrigin          = "origin"
 	keyJiraID          = "jira-id"
 	keyJiraOperationID = "jira-derived-id"
 	keyJiraKey         = "jira-key"
@@ -216,7 +215,7 @@ func (self *jiraImporter) ensureIssue(
 			cleanText,
 			nil,
 			map[string]string{
-				keyOrigin:      target,
+				core.KeyOrigin: target,
 				keyJiraID:      issue.ID,
 				keyJiraKey:     issue.Key,
 				keyJiraProject: self.conf[keyProject],
@@ -237,8 +236,8 @@ func getTimeDerivedID(jiraID string, timestamp MyTime) string {
 }
 
 // Create a bug.Comment from a JIRA comment
-func (self *jiraImporter) ensureComment(
-	repo *cache.RepoCache, b *cache.BugCache, item Comment) error {
+func (self *jiraImporter) ensureComment(repo *cache.RepoCache,
+	b *cache.BugCache, item Comment) error {
 	// ensure person
 	author, err := self.ensurePerson(repo, item.Author)
 	if err != nil {
@@ -271,8 +270,7 @@ func (self *jiraImporter) ensureComment(
 			cleanText,
 			nil,
 			map[string]string{
-				keyJiraID:      item.ID,
-				keyJiraProject: self.conf[keyProject],
+				keyJiraID: item.ID,
 			},
 		)
 		if err != nil {
@@ -315,8 +313,7 @@ func (self *jiraImporter) ensureComment(
 		target,
 		cleanText,
 		map[string]string{
-			keyJiraID:      derivedID,
-			keyJiraProject: self.conf[keyProject],
+			keyJiraID: derivedID,
 		},
 	)
 
@@ -489,7 +486,6 @@ func (self *jiraImporter) ensureChange(
 				map[string]string{
 					keyJiraID:          entry.ID,
 					keyJiraOperationID: derivedID,
-					keyJiraProject:     self.conf[keyProject],
 				},
 			)
 			if err != nil {
@@ -504,9 +500,7 @@ func (self *jiraImporter) ensureChange(
 					author,
 					entry.Created.Unix(),
 					map[string]string{
-						keyJiraID: entry.ID,
-
-						keyJiraProject:     self.conf[keyProject],
+						keyJiraID:          entry.ID,
 						keyJiraOperationID: derivedID,
 					},
 				)
@@ -519,9 +513,7 @@ func (self *jiraImporter) ensureChange(
 					author,
 					entry.Created.Unix(),
 					map[string]string{
-						keyJiraID: entry.ID,
-
-						keyJiraProject:     self.conf[keyProject],
+						keyJiraID:          entry.ID,
 						keyJiraOperationID: derivedID,
 					},
 				)
@@ -545,7 +537,6 @@ func (self *jiraImporter) ensureChange(
 				map[string]string{
 					keyJiraID:          entry.ID,
 					keyJiraOperationID: derivedID,
-					keyJiraProject:     self.conf[keyProject],
 				},
 			)
 			if err != nil {
@@ -564,7 +555,6 @@ func (self *jiraImporter) ensureChange(
 				map[string]string{
 					keyJiraID:          entry.ID,
 					keyJiraOperationID: derivedID,
-					keyJiraProject:     self.conf[keyProject],
 				},
 			)
 			if err != nil {