Merge pull request #66 from n4ss/release-v0.5.1

Update version tag to v0.5.1

MAINTAINERS

@@ -25,6 +25,7 @@
 			"lk4d4",
 			"mavenugo",
 			"mhbauer",
+			"n4ss",
 			"runcom",
 			"stevvooe",
 			"thajeztah",
@@ -114,6 +115,11 @@
 	Email = "mbauer@us.ibm.com"
 	GitHub = "mhbauer"
 
+	[people.n4ss]
+	Name = "Nassim Eddequiouaq"
+	Email = "eddequiouaq.nassim@gmail.com"
+	GitHub = "n4ss"
+
 	[people.runcom]
 	Name = "Antonio Murdaca"
 	Email = "runcom@redhat.com"

Makefile

@@ -1,7 +1,7 @@
 .PHONY: all deps osxkeychain secretservice test validate wincred
 
 TRAVIS_OS_NAME ?= linux
-VERSION = 0.5.0
+VERSION = 0.5.1
 
 all: test
 

client/client.go

@@ -9,12 +9,27 @@ import (
 	"github.com/docker/docker-credential-helpers/credentials"
 )
 
+// isValidCredsMessage checks if 'msg' contains invalid credentials error message.
+// It returns whether the logs are free of invalid credentials errors and the error if it isn't.
+// error values can be errCredentialsMissingServerURL or errCredentialsMissingUsername.
+func isValidCredsMessage(msg string) error {
+	if credentials.IsCredentialsMissingServerURLMessage(msg) {
+		return credentials.NewErrCredentialsMissingServerURL()
+	}
+
+	if credentials.IsCredentialsMissingUsernameMessage(msg) {
+		return credentials.NewErrCredentialsMissingUsername()
+	}
+
+	return nil
+}
+
 // Store uses an external program to save credentials.
-func Store(program ProgramFunc, credentials *credentials.Credentials) error {
+func Store(program ProgramFunc, creds *credentials.Credentials) error {
 	cmd := program("store")
 
 	buffer := new(bytes.Buffer)
-	if err := json.NewEncoder(buffer).Encode(credentials); err != nil {
+	if err := json.NewEncoder(buffer).Encode(creds); err != nil {
 		return err
 	}
 	cmd.Input(buffer)
@@ -22,6 +37,11 @@ func Store(program ProgramFunc, credentials *credentials.Credentials) error {
 	out, err := cmd.Output()
 	if err != nil {
 		t := strings.TrimSpace(string(out))
+
+		if isValidErr := isValidCredsMessage(t); isValidErr != nil {
+			err = isValidErr
+		}
+
 		return fmt.Errorf("error storing credentials - err: %v, out: `%s`", err, t)
 	}
 
@@ -41,6 +61,10 @@ func Get(program ProgramFunc, serverURL string) (*credentials.Credentials, error
 			return nil, credentials.NewErrCredentialsNotFound()
 		}
 
+		if isValidErr := isValidCredsMessage(t); isValidErr != nil {
+			err = isValidErr
+		}
+
 		return nil, fmt.Errorf("error getting credentials - err: %v, out: `%s`", err, t)
 	}
 
@@ -62,6 +86,11 @@ func Erase(program ProgramFunc, serverURL string) error {
 	out, err := cmd.Output()
 	if err != nil {
 		t := strings.TrimSpace(string(out))
+
+		if isValidErr := isValidCredsMessage(t); isValidErr != nil {
+			err = isValidErr
+		}
+
 		return fmt.Errorf("error erasing credentials - err: %v, out: `%s`", err, t)
 	}
 
@@ -75,6 +104,11 @@ func List(program ProgramFunc) (map[string]string, error) {
 	out, err := cmd.Output()
 	if err != nil {
 		t := strings.TrimSpace(string(out))
+
+		if isValidErr := isValidCredsMessage(t); isValidErr != nil {
+			err = isValidErr
+		}
+
 		return nil, fmt.Errorf("error listing credentials - err: %v, out: `%s`", err, t)
 	}
 

client/client_test.go

@@ -56,6 +56,8 @@ func (m *mockProgram) Output() ([]byte, error) {
 			return []byte(credentials.NewErrCredentialsNotFound().Error()), errProgramExited
 		case invalidServerAddress:
 			return []byte("program failed"), errProgramExited
+		case "":
+			return []byte(credentials.NewErrCredentialsMissingServerURL().Error()), errProgramExited
 		}
 	case "store":
 		var c credentials.Credentials
@@ -158,12 +160,16 @@ func TestGet(t *testing.T) {
 		}
 	}
 
+	missingServerURLErr := credentials.NewErrCredentialsMissingServerURL()
+
 	invalid := []struct {
 		serverURL string
 		err       string
 	}{
 		{missingCredsAddress, credentials.NewErrCredentialsNotFound().Error()},
 		{invalidServerAddress, "error getting credentials - err: exited 1, out: `program failed`"},
+		{"", fmt.Sprintf("error getting credentials - err: %s, out: `%s`",
+			missingServerURLErr.Error(), missingServerURLErr.Error())},
 	}
 
 	for _, v := range invalid {

client/command.go

@@ -2,6 +2,7 @@ package client
 
 import (
 	"io"
+	"os"
 	"os/exec"
 )
 
@@ -17,10 +18,16 @@ type ProgramFunc func(args ...string) Program
 // NewShellProgramFunc creates programs that are executed in a Shell.
 func NewShellProgramFunc(name string) ProgramFunc {
 	return func(args ...string) Program {
-		return &Shell{cmd: exec.Command(name, args...)}
+		return &Shell{cmd: newCmdRedirectErr(name, args)}
 	}
 }
 
+func newCmdRedirectErr(name string, args []string) *exec.Cmd {
+	newCmd := exec.Command(name, args...)
+	newCmd.Stderr = os.Stderr
+	return newCmd
+}
+
 // Shell invokes shell commands to talk with a remote credentials helper.
 type Shell struct {
 	cmd *exec.Cmd

credentials/credentials.go

@@ -17,6 +17,22 @@ type Credentials struct {
 	Secret    string
 }
 
+// isValid checks the integrity of Credentials object such that no credentials lack
+// a server URL or a username.
+// It returns whether the credentials are valid and the error if it isn't.
+// error values can be errCredentialsMissingServerURL or errCredentialsMissingUsername
+func (c *Credentials) isValid() (bool, error) {
+	if len(c.ServerURL) == 0 {
+		return false, NewErrCredentialsMissingServerURL()
+	}
+
+	if len(c.Username) == 0 {
+		return false, NewErrCredentialsMissingUsername()
+	}
+
+	return true, nil
+}
+
 // Docker credentials should be labeled as such in credentials stores that allow labelling.
 // That label allows to filter out non-Docker credentials too at lookup/search in macOS keychain,
 // Windows credentials manager and Linux libsecret. Default value is "Docker Credentials"
@@ -81,6 +97,10 @@ func Store(helper Helper, reader io.Reader) error {
 		return err
 	}
 
+	if ok, err := creds.isValid(); !ok {
+		return err
+	}
+
 	return helper.Add(&creds)
 }
 
@@ -100,6 +120,9 @@ func Get(helper Helper, reader io.Reader, writer io.Writer) error {
 	}
 
 	serverURL := strings.TrimSpace(buffer.String())
+	if len(serverURL) == 0 {
+		return NewErrCredentialsMissingServerURL()
+	}
 
 	username, secret, err := helper.Get(serverURL)
 	if err != nil {
@@ -107,6 +130,7 @@ func Get(helper Helper, reader io.Reader, writer io.Writer) error {
 	}
 
 	resp := Credentials{
+		ServerURL: serverURL,
 		Username: username,
 		Secret:   secret,
 	}
@@ -135,6 +159,9 @@ func Erase(helper Helper, reader io.Reader) error {
 	}
 
 	serverURL := strings.TrimSpace(buffer.String())
+	if len(serverURL) == 0 {
+		return NewErrCredentialsMissingServerURL()
+	}
 
 	return helper.Delete(serverURL)
 }

credentials/credentials_test.go

@@ -73,6 +73,46 @@ func TestStore(t *testing.T) {
 	}
 }
 
+func TestStoreMissingServerURL(t *testing.T) {
+	creds := &Credentials{
+		ServerURL: "",
+		Username: "foo",
+		Secret: "bar",
+	}
+
+	b, err := json.Marshal(creds)
+	if err != nil {
+		t.Fatal(err)
+	}
+	in := bytes.NewReader(b)
+
+	h := newMemoryStore()
+
+	if err := Store(h, in); IsCredentialsMissingServerURL(err) == false {
+		t.Fatal(err)
+	}
+}
+
+func TestStoreMissingUsername(t *testing.T) {
+	creds := &Credentials{
+		ServerURL: "https://index.docker.io/v1/",
+		Username: "",
+		Secret: "bar",
+	}
+
+	b, err := json.Marshal(creds)
+	if err != nil {
+		t.Fatal(err)
+	}
+	in := bytes.NewReader(b)
+
+	h := newMemoryStore()
+
+	if err := Store(h, in); IsCredentialsMissingUsername(err) == false {
+		t.Fatal(err)
+	}
+}
+
 func TestGet(t *testing.T) {
 	serverURL := "https://index.docker.io/v1/"
 	creds := &Credentials{
@@ -115,6 +155,32 @@ func TestGet(t *testing.T) {
 	}
 }
 
+func TestGetMissingServerURL(t *testing.T) {
+	serverURL := "https://index.docker.io/v1/"
+	creds := &Credentials{
+		ServerURL: serverURL,
+		Username:  "foo",
+		Secret:    "bar",
+	}
+	b, err := json.Marshal(creds)
+	if err != nil {
+		t.Fatal(err)
+	}
+	in := bytes.NewReader(b)
+
+	h := newMemoryStore()
+	if err := Store(h, in); err != nil {
+		t.Fatal(err)
+	}
+
+	buf := strings.NewReader("")
+	w := new(bytes.Buffer)
+
+	if err := Get(h, buf, w); IsCredentialsMissingServerURL(err) == false {
+		t.Fatal(err)
+	}
+}
+
 func TestErase(t *testing.T) {
 	serverURL := "https://index.docker.io/v1/"
 	creds := &Credentials{
@@ -144,6 +210,30 @@ func TestErase(t *testing.T) {
 	}
 }
 
+func TestEraseMissingServerURL(t *testing.T) {
+	serverURL := "https://index.docker.io/v1/"
+	creds := &Credentials{
+		ServerURL: serverURL,
+		Username:  "foo",
+		Secret:    "bar",
+	}
+	b, err := json.Marshal(creds)
+	if err != nil {
+		t.Fatal(err)
+	}
+	in := bytes.NewReader(b)
+
+	h := newMemoryStore()
+	if err := Store(h, in); err != nil {
+		t.Fatal(err)
+	}
+
+	buf := strings.NewReader("")
+	if err := Erase(h, buf); IsCredentialsMissingServerURL(err) == false {
+		t.Fatal(err)
+	}
+}
+
 func TestList(t *testing.T) {
 	//This tests that there is proper input an output into the byte stream
 	//Individual stores are very OS specific and have been tested in osxkeychain and secretservice respectively

credentials/error.go

@@ -1,8 +1,15 @@
 package credentials
 
-// ErrCredentialsNotFound standarizes the not found error, so every helper returns
+const (
+	// ErrCredentialsNotFound standardizes the not found error, so every helper returns
 	// the same message and docker can handle it properly.
-const errCredentialsNotFoundMessage = "credentials not found in native keychain"
+	errCredentialsNotFoundMessage = "credentials not found in native keychain"
+
+	// ErrCredentialsMissingServerURL and ErrCredentialsMissingUsername standardize
+	// invalid credentials or credentials management operations
+	errCredentialsMissingServerURLMessage = "no credentials server URL"
+	errCredentialsMissingUsernameMessage = "no credentials username"
+)
 
 // errCredentialsNotFound represents an error
 // raised when credentials are not in the store.
@@ -35,3 +42,64 @@ func IsErrCredentialsNotFound(err error) bool {
 func IsErrCredentialsNotFoundMessage(err string) bool {
 	return err == errCredentialsNotFoundMessage
 }
+
+
+// errCredentialsMissingServerURL represents an error raised
+// when the credentials object has no server URL or when no
+// server URL is provided to a credentials operation requiring
+// one.
+type errCredentialsMissingServerURL struct{}
+
+func (errCredentialsMissingServerURL) Error() string {
+	return errCredentialsMissingServerURLMessage
+}
+
+// errCredentialsMissingUsername represents an error raised
+// when the credentials object has no username or when no
+// username is provided to a credentials operation requiring
+// one.
+type errCredentialsMissingUsername struct{}
+
+func (errCredentialsMissingUsername) Error() string {
+	return errCredentialsMissingUsernameMessage
+}
+
+
+// NewErrCredentialsMissingServerURL creates a new error for
+// errCredentialsMissingServerURL.
+func NewErrCredentialsMissingServerURL() error {
+	return errCredentialsMissingServerURL{}
+}
+
+// NewErrCredentialsMissingUsername creates a new error for
+// errCredentialsMissingUsername.
+func NewErrCredentialsMissingUsername() error {
+	return errCredentialsMissingUsername{}
+}
+
+
+// IsCredentialsMissingServerURL returns true if the error
+// was an errCredentialsMissingServerURL.
+func IsCredentialsMissingServerURL(err error) bool {
+	_, ok := err.(errCredentialsMissingServerURL)
+	return ok
+}
+
+// IsCredentialsMissingServerURLMessage checks for an
+// errCredentialsMissingServerURL in the error message.
+func IsCredentialsMissingServerURLMessage(err string) bool {
+	return err == errCredentialsMissingServerURLMessage
+}
+
+// IsCredentialsMissingUsername returns true if the error
+// was an errCredentialsMissingUsername.
+func IsCredentialsMissingUsername(err error) bool {
+	_, ok := err.(errCredentialsMissingUsername)
+	return ok
+}
+
+// IsCredentialsMissingUsernameMessage checks for an
+// errCredentialsMissingUsername in the error message.
+func IsCredentialsMissingUsernameMessage(err string) bool {
+	return err == errCredentialsMissingUsernameMessage
+}

wincred/wincred_windows.go

@@ -2,9 +2,10 @@ package wincred
 
 import (
 	"bytes"
+	"strings"
+
 	winc "github.com/danieljoos/wincred"
 	"github.com/docker/docker-credential-helpers/credentials"
-	"strings"
 )
 
 // Wincred handles secrets using the Windows credential service.
@@ -39,8 +40,15 @@ func (h Wincred) Get(serverURL string) (string, string, error) {
 	if g == nil {
 		return "", "", credentials.NewErrCredentialsNotFound()
 	}
+	for _, attr := range g.Attributes {
+		if strings.Compare(attr.Keyword, "label") == 0 &&
+			bytes.Compare(attr.Value, []byte(credentials.CredsLabel)) == 0 {
+
 			return g.UserName, string(g.CredentialBlob), nil
 		}
+	}
+	return "", "", credentials.NewErrCredentialsNotFound()
+}
 
 // List returns the stored URLs and corresponding usernames for a given credentials label.
 func (h Wincred) List() (map[string]string, error) {