diff options
| -rw-r--r-- | bridge/core/interfaces.go | 22 | ||||
| -rw-r--r-- | bridge/github/config.go | 83 | ||||
| -rw-r--r-- | bridge/launchpad/config.go | 2 | ||||
| -rw-r--r-- | commands/bridge_auth_addtoken.go | 6 |
4 files changed, 80 insertions, 33 deletions
diff --git a/bridge/core/interfaces.go b/bridge/core/interfaces.go index f20f1642..47dbd63b 100644 --- a/bridge/core/interfaces.go +++ b/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 { diff --git a/bridge/github/config.go b/bridge/github/config.go index a7af3c02..61d641a6 100644 --- a/bridge/github/config.go +++ b/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) { diff --git a/bridge/launchpad/config.go b/bridge/launchpad/config.go index 0bf8dc0d..e2eb86da 100644 --- a/bridge/launchpad/config.go +++ b/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 } diff --git a/commands/bridge_auth_addtoken.go b/commands/bridge_auth_addtoken.go index 338b170e..8eda28ac 100644 --- a/commands/bridge_auth_addtoken.go +++ b/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") } |