Merge pull request #278 from thaJeztah/osxkeychain_typed_error

osxkeychain: Delete(): return typed errors
docker · May 27, 2023 · f09e79d · f09e79d
2 parents 9ff5b61 + b21b69c
commit f09e79d
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 18 deletions.
diff --git a/osxkeychain/osxkeychain_darwin.go b/osxkeychain/osxkeychain_darwin.go
@@ -34,7 +34,7 @@ type Osxkeychain struct{}
 
 // Add adds new credentials to the keychain.
 func (h Osxkeychain) Add(creds *credentials.Credentials) error {
-	h.Delete(creds.ServerURL)
+	_ = h.Delete(creds.ServerURL) // ignore errors as existing credential may not exist.
 
 	s, err := splitServer(creds.ServerURL)
 	if err != nil {
@@ -66,10 +66,16 @@ func (h Osxkeychain) Delete(serverURL string) error {
 	}
 	defer freeServer(s)
 
-	errMsg := C.keychain_delete(s)
-	if errMsg != nil {
+	if errMsg := C.keychain_delete(s); errMsg != nil {
 		defer C.free(unsafe.Pointer(errMsg))
-		return errors.New(C.GoString(errMsg))
+		switch goMsg := C.GoString(errMsg); goMsg {
+		case errCredentialsNotFound:
+			return credentials.NewErrCredentialsNotFound()
+		case errInteractionNotAllowed:
+			return ErrInteractionNotAllowed
+		default:
+			return errors.New(goMsg)
+		}
 	}
 
 	return nil
@@ -93,15 +99,14 @@ func (h Osxkeychain) Get(serverURL string) (string, string, error) {
 	errMsg := C.keychain_get(s, &usernameLen, &username, &secretLen, &secret)
 	if errMsg != nil {
 		defer C.free(unsafe.Pointer(errMsg))
-		goMsg := C.GoString(errMsg)
-		if goMsg == errCredentialsNotFound {
+		switch goMsg := C.GoString(errMsg); goMsg {
+		case errCredentialsNotFound:
 			return "", "", credentials.NewErrCredentialsNotFound()
-		}
-		if goMsg == errInteractionNotAllowed {
+		case errInteractionNotAllowed:
 			return "", "", ErrInteractionNotAllowed
+		default:
+			return "", "", errors.New(goMsg)
 		}
-
-		return "", "", errors.New(goMsg)
 	}
 
 	user := C.GoStringN(username, C.int(usernameLen))
@@ -124,15 +129,14 @@ func (h Osxkeychain) List() (map[string]string, error) {
 	defer C.freeListData(&acctsC, listLenC)
 	if errMsg != nil {
 		defer C.free(unsafe.Pointer(errMsg))
-		goMsg := C.GoString(errMsg)
-		if goMsg == errCredentialsNotFound {
+		switch goMsg := C.GoString(errMsg); goMsg {
+		case errCredentialsNotFound:
 			return make(map[string]string), nil
-		}
-		if goMsg == errInteractionNotAllowed {
+		case errInteractionNotAllowed:
 			return nil, ErrInteractionNotAllowed
+		default:
+			return nil, errors.New(goMsg)
 		}
-
-		return nil, errors.New(goMsg)
 	}
 
 	var listLen int

diff --git a/osxkeychain/osxkeychain_darwin_test.go b/osxkeychain/osxkeychain_darwin_test.go
@@ -205,9 +205,14 @@ func TestOSXKeychainHelperStoreRetrieve(t *testing.T) {
 }
 
 func TestMissingCredentials(t *testing.T) {
+	const nonExistingCred = "https://adsfasdf.invalid/asdfsdddd"
 	helper := Osxkeychain{}
-	_, _, err := helper.Get("https://adsfasdf.wrewerwer.com/asdfsdddd")
+	_, _, err := helper.Get(nonExistingCred)
 	if !credentials.IsErrCredentialsNotFound(err) {
-		t.Fatalf("expected ErrCredentialsNotFound, got %v", err)
+		t.Errorf("expected ErrCredentialsNotFound, got %v", err)
+	}
+	err = helper.Delete(nonExistingCred)
+	if !credentials.IsErrCredentialsNotFound(err) {
+		t.Errorf("expected ErrCredentialsNotFound, got %v", err)
 	}
 }