github: correct casing for user provided login

Michael Muré created

Change summary

bridge/core/interfaces.go        | 22 ++++----
bridge/github/config.go          | 83 ++++++++++++++++++++++++---------
bridge/launchpad/config.go       |  2 
commands/bridge_auth_addtoken.go |  6 ++
4 files changed, 80 insertions(+), 33 deletions(-)

Detailed changes

bridge/core/interfaces.go 🔗

@@ -13,26 +13,26 @@ type BridgeImpl interface {
 	// Target return the target of the bridge (e.g.: "github")
 	Target() string
 
-	// LoginMetaKey return the metadata key used to store the remote bug-tracker login
-	// on the user identity. The corresponding value is used to match identities and
-	// credentials.
-	LoginMetaKey() string
+	// NewImporter return an Importer implementation if the import is supported
+	NewImporter() Importer
 
-	// The set of the BridgeParams fields supported
-	ValidParams() map[string]interface{}
+	// NewExporter return an Exporter implementation if the export is supported
+	NewExporter() Exporter
 
 	// Configure handle the user interaction and return a key/value configuration
 	// for future use
 	Configure(repo *cache.RepoCache, params BridgeParams) (Configuration, error)
 
+	// The set of the BridgeParams fields supported
+	ValidParams() map[string]interface{}
+
 	// ValidateConfig check the configuration for error
 	ValidateConfig(conf Configuration) error
 
-	// NewImporter return an Importer implementation if the import is supported
-	NewImporter() Importer
-
-	// NewExporter return an Exporter implementation if the export is supported
-	NewExporter() Exporter
+	// LoginMetaKey return the metadata key used to store the remote bug-tracker login
+	// on the user identity. The corresponding value is used to match identities and
+	// credentials.
+	LoginMetaKey() string
 }
 
 type Importer interface {

bridge/github/config.go 🔗

@@ -42,6 +42,7 @@ func (g *Github) Configure(repo *cache.RepoCache, params core.BridgeParams) (cor
 	var err error
 	var owner string
 	var project string
+	var ok bool
 
 	// getting owner and project name
 	switch {
@@ -63,8 +64,8 @@ func (g *Github) Configure(repo *cache.RepoCache, params core.BridgeParams) (cor
 		}
 	}
 
-	// validate project owner
-	ok, err := validateUsername(owner)
+	// validate project owner and override with the correct case
+	ok, owner, err = validateUsername(owner)
 	if err != nil {
 		return nil, err
 	}
@@ -95,13 +96,18 @@ func (g *Github) Configure(repo *cache.RepoCache, params core.BridgeParams) (cor
 		token.SetMetadata(auth.MetaKeyLogin, login)
 		cred = token
 	default:
-		login = params.Login
-		if login == "" {
-			login, err = input.Prompt("Github login", "login", input.Required, usernameValidator)
-			if err != nil {
-				return nil, err
+		if params.Login == "" {
+			login, err = promptLogin()
+		} else {
+			// validate login and override with the correct case
+			ok, login, err = validateUsername(params.Login)
+			if !ok {
+				return nil, fmt.Errorf("invalid parameter login: %v", params.Login)
 			}
 		}
+		if err != nil {
+			return nil, err
+		}
 		cred, err = promptTokenOptions(repo, login, owner, project)
 		if err != nil {
 			return nil, err
@@ -163,17 +169,6 @@ func (*Github) ValidateConfig(conf core.Configuration) error {
 	return nil
 }
 
-func usernameValidator(_ string, value string) (string, error) {
-	ok, err := validateUsername(value)
-	if err != nil {
-		return "", err
-	}
-	if !ok {
-		return "invalid login", nil
-	}
-	return "", nil
-}
-
 func requestToken(note, login, password string, scope string) (*http.Response, error) {
 	return requestTokenWith2FA(note, login, password, "", scope)
 }
@@ -431,7 +426,30 @@ func getValidGithubRemoteURLs(repo repository.RepoCommon) ([]string, error) {
 	return urls, nil
 }
 
-func validateUsername(username string) (bool, error) {
+func promptLogin() (string, error) {
+	var login string
+
+	validator := func(_ string, value string) (string, error) {
+		ok, fixed, err := validateUsername(value)
+		if err != nil {
+			return "", err
+		}
+		if !ok {
+			return "invalid login", nil
+		}
+		login = fixed
+		return "", nil
+	}
+
+	_, err := input.Prompt("Github login", "login", input.Required, validator)
+	if err != nil {
+		return "", err
+	}
+
+	return login, nil
+}
+
+func validateUsername(username string) (bool, string, error) {
 	url := fmt.Sprintf("%s/users/%s", githubV3Url, username)
 
 	client := &http.Client{
@@ -440,15 +458,36 @@ func validateUsername(username string) (bool, error) {
 
 	resp, err := client.Get(url)
 	if err != nil {
-		return false, err
+		return false, "", err
+	}
+
+	if resp.StatusCode != http.StatusOK {
+		return false, "", nil
+	}
+
+	data, err := ioutil.ReadAll(resp.Body)
+	if err != nil {
+		return false, "", err
 	}
 
 	err = resp.Body.Close()
 	if err != nil {
-		return false, err
+		return false, "", err
 	}
 
-	return resp.StatusCode == http.StatusOK, nil
+	var decoded struct {
+		Login string `json:"login"`
+	}
+	err = json.Unmarshal(data, &decoded)
+	if err != nil {
+		return false, "", err
+	}
+
+	if decoded.Login == "" {
+		return false, "", fmt.Errorf("validateUsername: missing login in the response")
+	}
+
+	return true, decoded.Login, nil
 }
 
 func validateProject(owner, project string, token *auth.Token) (bool, error) {

bridge/launchpad/config.go 🔗

@@ -85,6 +85,8 @@ func validateProject(project string) (bool, error) {
 		return false, err
 	}
 
+	_ = resp.Body.Close()
+
 	return resp.StatusCode == http.StatusOK, nil
 }
 

commands/bridge_auth_addtoken.go 🔗

@@ -24,6 +24,12 @@ var (
 )
 
 func runBridgeTokenAdd(cmd *cobra.Command, args []string) error {
+	// Note: as bridgeAuthAddTokenLogin is not checked against the remote bug-tracker,
+	// it's possible to register a credential with an incorrect login (including bad case).
+	// The consequence is that it will not get picked later by the bridge. I find that
+	// checking it would require a cumbersome UX (need to provide a base URL for some bridges, ...)
+	// so it's probably not worth it, unless we refactor that entirely.
+
 	if bridgeAuthAddTokenTarget == "" {
 		return fmt.Errorf("flag --target is required")
 	}