Improve auth process

This is to follow implementation in https://github.com/github/hub/pull/738

github/client.go

@@ -5,6 +5,7 @@ import (
 	"io"
 	"net/url"
 	"os"
+	"os/user"
 	"strings"
 
 	"github.com/github/hub/Godeps/_workspace/src/github.com/octokit/go-octokit/octokit"
@@ -14,7 +15,6 @@ const (
 	GitHubHost    string = "github.com"
 	GitHubApiHost string = "api.github.com"
 	UserAgent     string = "Hub"
-	OAuthAppName  string = "hub"
 	OAuthAppURL   string = "http://hub.github.com/"
 )
 
@@ -27,18 +27,23 @@ func NewClientWithHost(host *Host) *Client {
 }
 
 type AuthError struct {
-	error
+	Err error
 }
 
 func (e *AuthError) Error() string {
-	return e.error.Error()
+	return e.Err.Error()
 }
 
-func (e *AuthError) Is2FAError() bool {
-	re, ok := e.error.(*octokit.ResponseError)
+func (e *AuthError) IsRequired2FACodeError() bool {
+	re, ok := e.Err.(*octokit.ResponseError)
 	return ok && re.Type == octokit.ErrorOneTimePasswordRequired
 }
 
+func (e *AuthError) IsDuplicatedTokenError() bool {
+	re, ok := e.Err.(*octokit.ResponseError)
+	return ok && re.Type == octokit.ErrorUnprocessableEntity
+}
+
 type Client struct {
 	Host *Host
 }
@@ -455,55 +460,39 @@ func (client *Client) FindOrCreateToken(user, password, twoFactorCode string) (t
 	c := client.newOctokitClient(basicAuth)
 	authsService := c.Authorizations(client.requestURL(authUrl))
 
-	if twoFactorCode == "" {
-		// dummy request to trigger a 2FA SMS since a HTTP GET won't do it
-		authsService.Create(nil)
+	authParam := octokit.AuthorizationParams{
+		Scopes:  []string{"repo"},
+		NoteURL: OAuthAppURL,
 	}
 
-	auths, result := authsService.All()
-	if result.HasError() {
-		err = &AuthError{result.Err}
-		return
-	}
-
-	var moreAuths []octokit.Authorization
-	for result.NextPage != nil {
-		authUrl, e := result.NextPage.Expand(nil)
+	count := 0
+	for {
+		note, e := authTokenNote(count)
 		if e != nil {
-			return "", e
-		}
-		authUrl, _ = url.Parse(authUrl.RequestURI())
-
-		as := c.Authorizations(authUrl)
-		moreAuths, result = as.All()
-		if result.HasError() {
-			err = &AuthError{result.Err}
+			err = e
 			return
 		}
 
-		auths = append(auths, moreAuths...)
-	}
-
-	for _, auth := range auths {
-		if auth.Note == OAuthAppName || auth.NoteURL == OAuthAppURL {
+		authParam.Note = note
+		auth, result := authsService.Create(authParam)
+		if !result.HasError() {
 			token = auth.Token
 			break
 		}
-	}
 
-	if token == "" {
-		authParam := octokit.AuthorizationParams{}
-		authParam.Scopes = append(authParam.Scopes, "repo")
-		authParam.Note = OAuthAppName
-		authParam.NoteURL = OAuthAppURL
-
-		auth, result := authsService.Create(authParam)
-		if result.HasError() {
-			err = &AuthError{result.Err}
-			return
+		authErr := &AuthError{result.Err}
+		if authErr.IsDuplicatedTokenError() {
+			if count >= 8 {
+				err = authErr
+				break
+			} else {
+				count++
+				continue
+			}
+		} else {
+			err = authErr
+			break
 		}
-
-		token = auth.Token
 	}
 
 	return
@@ -577,6 +566,8 @@ func FormatError(action string, err error) (ee error) {
 	switch e := err.(type) {
 	default:
 		ee = err
+	case *AuthError:
+		return FormatError(action, e.Err)
 	case *octokit.ResponseError:
 		statusCode := e.Response.StatusCode
 		var reason string
@@ -586,26 +577,33 @@ func FormatError(action string, err error) (ee error) {
 
 		errStr := fmt.Sprintf("Error %s: %s (HTTP %d)", action, reason, statusCode)
 
-		var messages []string
-		if statusCode == 422 {
-			if e.Message != "" {
-				messages = append(messages, e.Message)
+		var errorSentences []string
+		for _, err := range e.Errors {
+			switch err.Code {
+			case "custom":
+				errorSentences = append(errorSentences, err.Message)
+			case "missing_field":
+				errorSentences = append(errorSentences, fmt.Sprintf("Missing filed: \"%s\"", err.Field))
+			case "already_exists":
+				errorSentences = append(errorSentences, fmt.Sprintf("Duplicate value for \"%s\"", err.Field))
+			case "invalid":
+				errorSentences = append(errorSentences, fmt.Sprintf("Invalid value for \"%s\"", err.Field))
+			case "unauthorized":
+				errorSentences = append(errorSentences, fmt.Sprintf("Not allowed to change field \"%s\"", err.Field))
 			}
+		}
 
-			if len(e.Errors) > 0 {
-				for _, e := range e.Errors {
-					messages = append(messages, e.Error())
-				}
-			}
+		var errorMessage string
+		if len(errorSentences) > 0 {
+			errorMessage = strings.Join(errorSentences, "\n")
+		} else {
+			errorMessage = e.Message
 		}
 
-		if len(messages) > 0 {
-			errStr = fmt.Sprintf("%s\n%s", errStr, strings.Join(messages, "\n"))
+		if errorMessage != "" {
+			errStr = fmt.Sprintf("%s\n%s", errStr, errorMessage)
 		}
 
-		ee = fmt.Errorf(errStr)
-	case *AuthError:
-		errStr := fmt.Sprintf("Error %s: Unauthorized (HTTP 401)", action)
 		ee = fmt.Errorf(errStr)
 	}
 
@@ -625,3 +623,22 @@ func warnExistenceOfRepo(project *Project, ee error) (err error) {
 
 	return
 }
+
+func authTokenNote(num int) (string, error) {
+	u, err := user.Current()
+	if err != nil {
+		return "", err
+	}
+
+	n := u.Username
+	h, err := os.Hostname()
+	if err != nil {
+		return "", err
+	}
+
+	if num > 0 {
+		return fmt.Sprintf("hub for %s@%s %d", n, h, num), nil
+	}
+
+	return fmt.Sprintf("hub for %s@%s", n, h), nil
+}

github/client_test.go

@@ -3,6 +3,7 @@ package github
 import (
 	"fmt"
 	"net/http"
+	"regexp"
 	"testing"
 
 	"github.com/github/hub/Godeps/_workspace/src/github.com/bmizerany/assert"
@@ -62,3 +63,18 @@ func TestClient_warnExistenceOfRepo(t *testing.T) {
 	err := warnExistenceOfRepo(project, e)
 	assert.Equal(t, "Are you sure that github.com/github/hub exists?", fmt.Sprintf("%s", err))
 }
+
+func TestAuthTokenNote(t *testing.T) {
+	note, err := authTokenNote(0)
+	assert.Equal(t, nil, err)
+
+	reg := regexp.MustCompile("hub for (.+)@(.+)")
+	assert.T(t, reg.MatchString(note))
+
+	note, err = authTokenNote(2)
+	assert.Equal(t, nil, err)
+
+	reg = regexp.MustCompile("hub for (.+)@(.+) 2")
+	assert.T(t, reg.MatchString(note))
+
+}

github/config.go

@@ -8,8 +8,8 @@ import (
 	"path/filepath"
 	"strconv"
 
-	"github.com/github/hub/utils"
 	"github.com/github/hub/Godeps/_workspace/src/github.com/howeyc/gopass"
+	"github.com/github/hub/utils"
 )
 
 var (
@@ -45,13 +45,18 @@ func (c *Config) PromptForHost(host string) (h *Host, err error) {
 	pass := c.PromptForPassword(host, user)
 
 	client := NewClient(host)
-	token, e := client.FindOrCreateToken(user, pass, "")
-	if e != nil {
-		if ae, ok := e.(*AuthError); ok && ae.Is2FAError() {
-			code := c.PromptForOTP()
-			token, err = client.FindOrCreateToken(user, pass, code)
+	var code, token string
+	for {
+		token, err = client.FindOrCreateToken(user, pass, code)
+		if err == nil {
+			break
+		}
+
+		if ae, ok := err.(*AuthError); ok && ae.IsRequired2FACodeError() {
+			fmt.Fprintln(os.Stderr, "warning: invalid two-factor code")
+			code = c.PromptForOTP()
 		} else {
-			err = e
+			break
 		}
 	}