avoid to swallow errors

Signed-off-by: Angelo De Caro <adc@zurich.ibm.com>

Author: adecaro

token/services/identity/membership/lm.go

@@ -430,18 +430,35 @@ func (l *LocalMembership) registerLocalIdentity(ctx context.Context, identityCon
 	l.logger.DebugfContext(ctx, "try to load identity with [%d] key managers [%v]", len(l.KeyManagerProviders), l.KeyManagerProviders)
 	for i, p := range l.KeyManagerProviders {
 		var err error
-		keyManager, err = p.Get(ctx, identityConfig)
-		if err == nil && keyManager != nil && len(keyManager.EnrollmentID()) != 0 {
-			priority = i
-			break
+		var km KeyManager
+		km, err = p.Get(ctx, identityConfig)
+		if err != nil {
+			errs = append(errs, err)
+			continue
+		}
+
+		if len(km.EnrollmentID()) == 0 {
+			errs = append(errs, errors.Errorf("no enrollment id found for identity [%s]", identityConfig.ID))
+			continue
 		}
-		keyManager = nil
-		errs = append(errs, err)
+
+		// only assign keyManager if the provider returned a valid enrollment id
+		keyManager = km
+		priority = i
+		break
 	}
 	if keyManager == nil {
-		return errors.Wrapf(
-			errors.Join(errs...),
-			"failed to get a key manager for the passed identity config for [%s:%s]",
+		logger.Errorf("no key manager found for identity [%s], err [%+v]", identityConfig.ID, errs)
+		err := errors.Join(errs...)
+		if err != nil {
+			return errors.Wrapf(err,
+				"failed to get a key manager for the passed identity config for [%s:%s]",
+				identityConfig.ID,
+				identityConfig.URL,
+			)
+		}
+		return errors.Errorf(
+			"no key manager found for [%s:%s]",
 			identityConfig.ID,
 			identityConfig.URL,
 		)
@@ -467,21 +484,29 @@ func (l *LocalMembership) registerLocalIdentity(ctx context.Context, identityCon
 func (l *LocalMembership) registerIdentityConfiguration(ctx context.Context, identity *IdentityConfiguration, defaultIdentity bool) error {
 	// Try to register the local identity
 	identity.URL = l.config.TranslatePath(identity.URL)
-	if err := l.registerLocalIdentity(ctx, identity, defaultIdentity); err != nil {
-		l.logger.Warnf("failed to load local identity at [%s]:[%s]", identity.URL, err)
+	err1 := l.registerLocalIdentity(ctx, identity, defaultIdentity)
+	if err1 == nil {
+		// nothing else needs to be done
+		return nil
+	}
+
+	// second chance, load the path as folder
+	{
+		l.logger.Warnf("failed to load local identity at [%s]:[%s]", identity.URL, err1)
 		// Does path correspond to a folder containing multiple identities?
-		if err := l.registerLocalIdentities(ctx, identity); err != nil {
-			return errors.WithMessagef(err, "failed to register local identity")
+		err2 := l.registerLocalIdentities(ctx, identity)
+		if err2 != nil {
+			return errors.Wrap(errors.Join(err1, err2), "failed to register local identity")
 		}
 	}
+
 	return nil
 }
 
 func (l *LocalMembership) registerLocalIdentities(ctx context.Context, configuration *IdentityConfiguration) error {
 	entries, err := os.ReadDir(configuration.URL)
 	if err != nil {
-		l.logger.Warnf("failed reading from [%s]: [%s]", configuration.URL, err)
-		return nil
+		return errors.Wrapf(err, "no valid identities found in [%s]", configuration.URL)
 	}
 	found := 0
 	var errs []error
@@ -502,7 +527,7 @@ func (l *LocalMembership) registerLocalIdentities(ctx context.Context, configura
 		found++
 	}
 	if found == 0 {
-		return errors.Errorf("no valid identities found in [%s], errs [%v]", configuration.URL, errs)
+		return errors.Wrapf(errors.Join(errs...), "no valid identities found in [%s]", configuration.URL)
 	}
 	return nil
 }

token/services/identity/membership/lm_test.go

@@ -251,13 +251,65 @@ func TestRegisterIdentity_KeyManagerProviderFails(t *testing.T) {
 		kmp,
 	)
 
-	// RegisterIdentity swallows register errors and tries to read the path; it returns nil
+	// Now RegisterIdentity returns an error when no key manager provider succeeds
 	err := lm.RegisterIdentity(ctx, membership.IdentityConfiguration{ID: "X", URL: "/tmp/x"})
-	assert.NoError(t, err)
+	assert.Error(t, err)
 	// nothing persisted
 	assert.Equal(t, 0, iss.AddConfigurationCallCount())
 }
 
+func TestRegisterIdentity_EmptyEnrollmentID(t *testing.T) {
+	ctx := t.Context()
+
+	ip := &mock.IdentityProvider{}
+	// no binds expected
+
+	des := &mock.SignerDeserializerManager{}
+
+	iss := &mock.IdentityStoreService{}
+	iss.ConfigurationExistsReturns(false, nil)
+	iss.AddConfigurationReturns(nil)
+
+	km := &mock.KeyManager{}
+	km.EnrollmentIDReturns("")
+	km.AnonymousReturns(false)
+	km.IsRemoteReturns(false)
+	km.IdentityTypeReturns("typ")
+
+	kmp := &mock.KeyManagerProvider{}
+	kmp.GetReturns(km, nil)
+
+	lm := membership.NewLocalMembership(
+		logging.MustGetLogger("test"),
+		&mock.Config{},
+		[]byte("netid"),
+		des,
+		iss,
+		"testType",
+		false,
+		ip,
+		kmp,
+	)
+
+	// Use a non-existent path so registerLocalIdentities will attempt to read it and return nil
+	nonExistent := filepath.Join(t.TempDir(), "does_not_exist")
+	// Now RegisterIdentity returns an error when the selected KeyManager has an empty EnrollmentID
+	err := lm.RegisterIdentity(ctx, membership.IdentityConfiguration{ID: "z", URL: nonExistent})
+	assert.Error(t, err)
+
+	// Because enrollment ID was empty, no configuration should be persisted
+	assert.Equal(t, 0, iss.AddConfigurationCallCount())
+
+	// Deserializer manager should not be called because no valid key manager was found
+	assert.Equal(t, 0, des.AddTypedSignerDeserializerCallCount())
+
+	// Identity provider should not be bound
+	assert.Equal(t, 0, ip.BindCallCount())
+
+	// KeyManagerProvider should have been called at least once
+	assert.GreaterOrEqual(t, kmp.GetCallCount(), 1)
+}
+
 func TestRegisterIdentity_BindFails(t *testing.T) {
 	ctx := t.Context()
 
@@ -292,9 +344,9 @@ func TestRegisterIdentity_BindFails(t *testing.T) {
 		kmp,
 	)
 
-	// RegisterIdentity swallows register errors (logs them) and returns nil
+	// Now RegisterIdentity returns an error when the bind fails during addLocalIdentity
 	err := lm.RegisterIdentity(ctx, membership.IdentityConfiguration{ID: "Y", URL: "/tmp/y"})
-	assert.NoError(t, err)
+	assert.Error(t, err)
 	// bind failed so nothing persisted
 	assert.Equal(t, 0, iss.AddConfigurationCallCount())
 }

token/services/identity/x509/crypto/configbuilder.go

@@ -25,9 +25,12 @@ const (
 func LoadConfig(dir string, keyStoreDirName string) (*Config, error) {
 	signcertDir := filepath.Join(dir, SignCertsDirName)
 	signcert, err := getPemMaterialFromDir(signcertDir)
-	if err != nil || len(signcert) == 0 {
+	if err != nil {
 		return nil, errors.Wrapf(err, "could not load a valid signer certificate from directory %s", signcertDir)
 	}
+	if len(signcert) == 0 {
+		return nil, errors.Errorf("no signer certificate found in directory %s", signcertDir)
+	}
 	// load secret key, if available. If not available, the public's key SKI will be used to load the secret key from the key store
 	if len(keyStoreDirName) == 0 {
 		keyStoreDirName = KeyStoreDirName

token/services/identity/x509/km.go

@@ -67,23 +67,27 @@ func NewKeyManagerFromConf(
 		var err error
 		conf, err = crypto.LoadConfig(configPath, keyStoreDirName)
 		if err != nil {
+			logger.Errorf("failed loading x509 configuration [%+v]", err)
 			return nil, nil, errors.WithMessagef(err, "could not get config from dir [%s]", configPath)
 		}
 	}
+	logger.Debugf("load x509 config, check version...")
 	// enforce version
 	if conf.Version != crypto.ProtobufProtocolVersionV1 {
 		return nil, nil, errors.Errorf("unsupported protocol version: %d", conf.Version)
 	}
+	logger.Debugf("load x509 config, new signing key manager...")
 	p, err := newSigningKeyManager(conf, bccspConfig, keyStore)
 	if err == nil {
+		logger.Debugf("load x509 config, new signing key manager...done [%v]", p)
 		return p, conf, nil
 	}
 	// load as verify only
 	p, conf, err = newVerifyingKeyManager(conf, bccspConfig)
 	if err != nil {
 		return nil, nil, err
 	}
-	return p, conf, err
+	return p, conf, nil
 }
 
 func newSigningKeyManager(conf *crypto.Config, bccspConfig *crypto.BCCSP, keyStore crypto.KeyStore) (*KeyManager, error) {

token/services/identity/x509/kmp.go

@@ -102,6 +102,8 @@ func (k *KeyManagerProvider) registerProvider(ctx context.Context, conf *crypto.
 		}
 	}
 
+	logger.DebugfContext(ctx, "prepare opts and conf for [%s][%s]", translatedPath, keyStorePath)
+
 	optsRaw, err := yaml.Marshal(identityConfig.Opts)
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed to marshal config [%v]", identityConfig)
@@ -113,6 +115,8 @@ func (k *KeyManagerProvider) registerProvider(ctx context.Context, conf *crypto.
 	idConfig.Config = optsRaw
 	idConfig.Raw = confRaw
 
+	logger.DebugfContext(ctx, "provider ready for [%s][%s][%s]", translatedPath, keyStorePath, provider)
+
 	return provider, nil
 }