OCM-4965: Secure store tweaks (#905)

Author: tylercreller

authentication/securestore/main.go

@@ -5,33 +5,32 @@ import (
 	"compress/gzip"
 	"fmt"
 	"io"
+	"strings"
 
 	"github.com/99designs/keyring"
 )
 
 const (
-	SecureStoreConfigKey = "securestore"       // OCM_CONFIG key to enable secure OS store
 	KindInternetPassword = "Internet password" // MacOS Keychain item kind
 	ItemKey              = "RedHatSSO"
 	CollectionName       = "login" // Common OS default collection name
 	MaxWindowsByteSize   = 2500    // Windows Credential Manager has a 2500 byte limit
 )
 
 var (
-	ErrNoBackendsAvailable = fmt.Errorf("no backends available, expected one of %v", allowedBackends)
-	// The order of the backends is important. The first backend in the list is the first one
-	// that will attempt to be used.
-	allowedBackends = []keyring.BackendType{
-		keyring.WinCredBackend,
-		keyring.KeychainBackend,
-		keyring.SecretServiceBackend,
-		keyring.PassBackend,
+	ErrKeyringUnavailable = fmt.Errorf("keyring is valid but is not available on the current OS")
+	ErrKeyringInvalid     = fmt.Errorf("keyring is invalid, expected one of: [%v]", strings.Join(AllowedBackends, ", "))
+	AllowedBackends       = []string{
+		string(keyring.WinCredBackend),
+		string(keyring.KeychainBackend),
+		string(keyring.SecretServiceBackend),
+		string(keyring.PassBackend),
 	}
 )
 
-func getKeyringConfig() keyring.Config {
+func getKeyringConfig(backend string) keyring.Config {
 	return keyring.Config{
-		AllowedBackends: allowedBackends,
+		AllowedBackends: []keyring.BackendType{keyring.BackendType(backend)},
 		// Generic
 		ServiceName: ItemKey,
 		// MacOS
@@ -46,35 +45,51 @@ func getKeyringConfig() keyring.Config {
 	}
 }
 
-// AvailableBackends provides a slice of all available backend keys on the current OS.
+// IsBackendAvailable provides validation that the desired backend is available on the current OS.
 //
-// Note: CGO_ENABLED=1 is required for OSX Keychain and darwin builds
+// Note: CGO_ENABLED=1 is required for darwin builds (enables OSX Keychain)
+func IsBackendAvailable(backend string) (isAvailable bool) {
+	if backend == "" {
+		return false
+	}
+
+	for _, avail := range AvailableBackends() {
+		if avail == backend {
+			isAvailable = true
+			break
+		}
+	}
+
+	return isAvailable
+}
+
+// AvailableBackends provides a slice of all available backend keys on the current OS.
 //
-// The first backend in the slice is the first one that will be used.
+// Note: CGO_ENABLED=1 is required for darwin builds (enables OSX Keychain)
 func AvailableBackends() []string {
 	b := []string{}
 
 	// Intersection between available backends from OS and allowed backends
 	for _, avail := range keyring.AvailableBackends() {
-		for _, allowed := range allowedBackends {
-			if avail == allowed {
-				b = append(b, string(allowed))
+		for _, allowed := range AllowedBackends {
+			if string(avail) == allowed {
+				b = append(b, allowed)
 			}
 		}
 	}
 
 	return b
 }
 
-// UpsertConfigToKeyring will upsert the provided credentials to first priority OS secure store.
+// UpsertConfigToKeyring will upsert the provided credentials to the desired OS secure store.
 //
-// Note: CGO_ENABLED=1 is required for OSX Keychain and darwin builds
-func UpsertConfigToKeyring(creds []byte) error {
-	if err := validateBackends(); err != nil {
+// Note: CGO_ENABLED=1 is required for darwin builds (enables OSX Keychain)
+func UpsertConfigToKeyring(backend string, creds []byte) error {
+	if err := ValidateBackend(backend); err != nil {
 		return err
 	}
 
-	ring, err := keyring.Open(getKeyringConfig())
+	ring, err := keyring.Open(getKeyringConfig(backend))
 	if err != nil {
 		return err
 	}
@@ -86,7 +101,7 @@ func UpsertConfigToKeyring(creds []byte) error {
 
 	// check if available backend contains windows credential manager and exceeds the byte limit
 	if len(compressed) > MaxWindowsByteSize &&
-		keyring.AvailableBackends()[0] == keyring.WinCredBackend {
+		backend == string(keyring.WinCredBackend) {
 		return fmt.Errorf("credentials are too large for Windows Credential Manager: %d bytes (max %d)", len(compressed), MaxWindowsByteSize)
 	}
 
@@ -103,32 +118,42 @@ func UpsertConfigToKeyring(creds []byte) error {
 // RemoveConfigFromKeyring will remove the credentials from the first priority OS secure store.
 //
 // Note: CGO_ENABLED=1 is required for OSX Keychain and darwin builds
-func RemoveConfigFromKeyring() error {
-	if err := validateBackends(); err != nil {
+func RemoveConfigFromKeyring(backend string) error {
+	if err := ValidateBackend(backend); err != nil {
 		return err
 	}
 
-	ring, err := keyring.Open(getKeyringConfig())
+	ring, err := keyring.Open(getKeyringConfig(backend))
 	if err != nil {
 		return err
 	}
 
 	err = ring.Remove(ItemKey)
+	if err != nil {
+		if err == keyring.ErrKeyNotFound {
+			// Ignore not found errors, key is already removed
+			return nil
+		}
+
+		if strings.Contains(err.Error(), "Keychain Error. (-25244)") {
+			return fmt.Errorf("%s\nThis application may not have permission to delete from the Keychain. Please check the permissions in the Keychain and try again", err.Error())
+		}
+	}
 
 	return err
 }
 
 // GetConfigFromKeyring will retrieve the credentials from the first priority OS secure store.
 //
-// Note: CGO_ENABLED=1 is required for OSX Keychain and darwin builds
-func GetConfigFromKeyring() ([]byte, error) {
-	if err := validateBackends(); err != nil {
+// Note: CGO_ENABLED=1 is required for darwin builds (enables OSX Keychain)
+func GetConfigFromKeyring(backend string) ([]byte, error) {
+	if err := ValidateBackend(backend); err != nil {
 		return nil, err
 	}
 
 	credentials := []byte("")
 
-	ring, err := keyring.Open(getKeyringConfig())
+	ring, err := keyring.Open(getKeyringConfig(backend))
 	if err != nil {
 		return nil, err
 	}
@@ -156,11 +181,29 @@ func GetConfigFromKeyring() ([]byte, error) {
 
 }
 
-// Validates that at least one backend is available
-func validateBackends() error {
-	if len(AvailableBackends()) == 0 {
-		return ErrNoBackendsAvailable
+// Validates that the requested backend is valid and available, returns an error if not.
+//
+// Note: CGO_ENABLED=1 is required for darwin builds (enables OSX Keychain)
+func ValidateBackend(backend string) error {
+	if backend == "" {
+		return ErrKeyringInvalid
+	} else {
+		isAllowedBackend := false
+		for _, allowed := range AllowedBackends {
+			if allowed == backend {
+				isAllowedBackend = true
+				break
+			}
+		}
+		if !isAllowedBackend {
+			return ErrKeyringInvalid
+		}
+	}
+
+	if !IsBackendAvailable(backend) {
+		return ErrKeyringUnavailable
 	}
+
 	return nil
 }
 

authentication/securestore/main_test.go

@@ -0,0 +1,65 @@
+/*
+Copyright (c) 2024 Red Hat, Inc.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// This file contains tests for the functions that extract authentication and authorization
+// information from contexts.
+
+package securestore
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo/v2/dsl/core" // nolint
+	. "github.com/onsi/gomega"             // nolint
+)
+
+func TestSecurestore(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Authentication.Securestore")
+}
+
+var _ = Describe("Validation", func() {
+	It("Validates an invalid backend", func() {
+		err := ValidateBackend("invalid")
+		Expect(err).To(Equal(ErrKeyringInvalid))
+
+		err = UpsertConfigToKeyring("invalid", []byte("test"))
+		Expect(err).To(Equal(ErrKeyringInvalid))
+
+		bytes, err := GetConfigFromKeyring("invalid")
+		Expect(err).To(Equal(ErrKeyringInvalid))
+		Expect(bytes).To(BeNil())
+
+		err = RemoveConfigFromKeyring("invalid")
+		Expect(err).To(Equal(ErrKeyringInvalid))
+	})
+
+	It("Validates an empty backend", func() {
+		err := ValidateBackend("")
+		Expect(err).To(Equal(ErrKeyringInvalid))
+
+		err = UpsertConfigToKeyring("", []byte("test"))
+		Expect(err).To(Equal(ErrKeyringInvalid))
+
+		bytes, err := GetConfigFromKeyring("")
+		Expect(err).To(Equal(ErrKeyringInvalid))
+		Expect(bytes).To(BeNil())
+
+		err = RemoveConfigFromKeyring("")
+		Expect(err).To(Equal(ErrKeyringInvalid))
+	})
+
+})

examples/secure_store.go

@@ -9,6 +9,10 @@ import (
 )
 
 func main() {
+
+	// Modify the following variable to match your OS:
+	keyring := "keychain"
+
 	// Create a context:
 	clientId := "ocm-cli"
 
@@ -27,22 +31,22 @@ func main() {
 	config := []byte("mybytestringagain")
 
 	// Upsert to keyring
-	err = securestore.UpsertConfigToKeyring(config)
+	err = securestore.UpsertConfigToKeyring(keyring, config)
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "Error upserting to keyring: %v", err)
 		os.Exit(1)
 	}
 
 	// Upsert again to keyring
 	config = []byte(token)
-	err = securestore.UpsertConfigToKeyring(config)
+	err = securestore.UpsertConfigToKeyring(keyring, config)
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "Error upserting to keyring: %v", err)
 		os.Exit(1)
 	}
 
 	// Read bytes back from Keyring
-	readVal, err := securestore.GetConfigFromKeyring()
+	readVal, err := securestore.GetConfigFromKeyring(keyring)
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "Error reading from keyring: %v", err)
 		os.Exit(1)