From 1ed95cb02025b035d21445f05e2e083319ff881d Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 27 May 2023 16:45:40 +0200 Subject: [PATCH 1/3] osxkeychain: explicitly ignore error Signed-off-by: Sebastiaan van Stijn --- osxkeychain/osxkeychain_darwin.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osxkeychain/osxkeychain_darwin.go b/osxkeychain/osxkeychain_darwin.go index 07432e6..33796cf 100644 --- 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 { From 7f00c5c8bdd6709f5b604aa0ef5e145af3ccd4f8 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 27 May 2023 16:48:48 +0200 Subject: [PATCH 2/3] osxkeychain: use a switch for handling errors Makes the error-handling slightly cleaner. Signed-off-by: Sebastiaan van Stijn --- osxkeychain/osxkeychain_darwin.go | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/osxkeychain/osxkeychain_darwin.go b/osxkeychain/osxkeychain_darwin.go index 33796cf..8d8925d 100644 --- a/osxkeychain/osxkeychain_darwin.go +++ b/osxkeychain/osxkeychain_darwin.go @@ -93,15 +93,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 +123,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 From b21b69c8ee306f4696859d6a2980bb0b4224539a Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 27 May 2023 16:06:22 +0200 Subject: [PATCH 3/3] osxkeychain: Delete(): return typed errors This allows a Delete for non-existing credentials to be handled. Signed-off-by: Sebastiaan van Stijn --- osxkeychain/osxkeychain_darwin.go | 12 +++++++++--- osxkeychain/osxkeychain_darwin_test.go | 9 +++++++-- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/osxkeychain/osxkeychain_darwin.go b/osxkeychain/osxkeychain_darwin.go index 8d8925d..272b763 100644 --- a/osxkeychain/osxkeychain_darwin.go +++ b/osxkeychain/osxkeychain_darwin.go @@ -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 diff --git a/osxkeychain/osxkeychain_darwin_test.go b/osxkeychain/osxkeychain_darwin_test.go index 27518e5..b5098ef 100644 --- 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) } }