Prevent invalid credentials: no missing server URL or username (#62)

* Prevent invalid credentials: no missing server URL or username Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com> * Add missing username/serverURL error checks in credstore client Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com> * Clean up doc on invalid creds errors and client's error checks Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com> * Add tests for missing ServerURL/Username Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com> * Clean isValidCredsMessage prototype Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com> * Add test for missing server URL and more detailed error msg Signed-off-by: Nassim 'Nass' Eddequiouaq <eddequiouaq.nassim@gmail.com>
2026-06-13 16:01:28 +05:30 · 2017-05-29 10:21:50 +02:00
parent 79ab7059b0
commit ed11c58ebf
5 changed files with 230 additions and 5 deletions
@@ -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)
 }
@@ -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
@@ -1,8 +1,15 @@
 package credentials

-// ErrCredentialsNotFound standarizes 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"
+const (
+	// ErrCredentialsNotFound standardizes the not found error, so every helper returns
+	// the same message and docker can handle it properly.
+	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
+}