aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--bridge/core/interfaces.go22
-rw-r--r--bridge/github/config.go83
-rw-r--r--bridge/github/config_test.go55
-rw-r--r--bridge/launchpad/config.go2
-rw-r--r--commands/bridge_auth_addtoken.go6
5 files changed, 112 insertions, 56 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/github/config_test.go b/bridge/github/config_test.go
index fe54c209..eecb1aa8 100644
--- a/bridge/github/config_test.go
+++ b/bridge/github/config_test.go
@@ -104,41 +104,50 @@ func TestValidateUsername(t *testing.T) {
t.Skip("Travis environment: avoiding non authenticated requests")
}
- type args struct {
- username string
- }
tests := []struct {
- name string
- args args
- want bool
+ name string
+ input string
+ fixed string
+ ok bool
}{
{
- name: "existing username",
- args: args{
- username: "MichaelMure",
- },
- want: true,
+ name: "existing username",
+ input: "MichaelMure",
+ fixed: "MichaelMure",
+ ok: true,
},
{
- name: "existing organisation name",
- args: args{
- username: "ipfs",
- },
- want: true,
+ name: "existing username with bad case",
+ input: "MicHaelmurE",
+ fixed: "MichaelMure",
+ ok: true,
},
{
- name: "non existing username",
- args: args{
- username: "cant-find-this",
- },
- want: false,
+ name: "existing organisation",
+ input: "ipfs",
+ fixed: "ipfs",
+ ok: true,
+ },
+ {
+ name: "existing organisation with bad case",
+ input: "iPfS",
+ fixed: "ipfs",
+ ok: true,
+ },
+ {
+ name: "non existing username",
+ input: "cant-find-this",
+ fixed: "",
+ ok: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
- ok, _ := validateUsername(tt.args.username)
- assert.Equal(t, tt.want, ok)
+ ok, fixed, err := validateUsername(tt.input)
+ assert.NoError(t, err)
+ assert.Equal(t, tt.ok, ok)
+ assert.Equal(t, tt.fixed, fixed)
})
}
}
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")
}