Fixes external auth privider removal issues

system/types/app_settings.go

@@ -310,7 +310,12 @@ func (set *ExternalAuthProviderSet) DecodeKV(kv SettingsKV, prefix string) (err
 	}
 
 	// create standard provider set
-	providers := map[string]bool{"github": true, "facebook": true, "google": true, "linkedin": true}
+	permanent := map[string]bool{"github": true, "facebook": true, "google": true, "linkedin": true}
+	// and make a working copy
+	providers := make(map[string]bool)
+	for k, v := range permanent {
+		providers[k] = v
+	}
 
 	// remove prefix
 	kv = kv.CutPrefix(prefix + ".")
@@ -340,6 +345,14 @@ func (set *ExternalAuthProviderSet) DecodeKV(kv SettingsKV, prefix string) (err
 		if err != nil {
 			return
 		}
+	}
+
+	// Cleanup
+	var clean = ExternalAuthProviderSet{}
+	for _, p := range *set {
+		if p.empty() && !permanent[p.Handle] {
+			continue
+		}
 
 		if p.Label == "" {
 			switch p.Handle {
@@ -348,8 +361,10 @@ func (set *ExternalAuthProviderSet) DecodeKV(kv SettingsKV, prefix string) (err
 			case "linkedin":
 				p.Label = "LinkedIn"
 			case "corteza-iam", "corteza", "corteza-one":
+				// Some legacy provider naming
 				p.Label = "Corteza IAM"
 			case "crust-iam", "crust", "crust-unify":
+				// Some legacy provider naming
 				p.Label = "Crust IAM"
 			default:
 				if strings.HasPrefix(p.Handle, oidcProviderPrefix) {
@@ -359,8 +374,12 @@ func (set *ExternalAuthProviderSet) DecodeKV(kv SettingsKV, prefix string) (err
 				}
 			}
 		}
+
+		clean = append(clean, p)
 	}
 
+	*set = clean
+
 	return
 }
 
@@ -393,7 +412,7 @@ func (set ExternalAuthProviderSet) Less(i, j int) bool {
 // Returns enabled providers, sorted with their redirect-URLs set...
 func (set ExternalAuthProviderSet) Valid() (out ExternalAuthProviderSet) {
 	for _, eap := range set {
-		if !eap.Enabled {
+		if eap.empty() || !eap.Enabled {
 			continue
 		}
 
@@ -405,21 +424,21 @@ func (set ExternalAuthProviderSet) Valid() (out ExternalAuthProviderSet) {
 
 var _ KVDecoder = &ExternalAuthProviderSet{}
 
-func (set ExternalAuthProvider) EncodeKV() (vv SettingValueSet, err error) {
-	if set.Handle == "" {
+func (eap ExternalAuthProvider) EncodeKV() (vv SettingValueSet, err error) {
+	if eap.Handle == "" {
 		return nil, errors.New("cannot encode external auth provider without handle")
 	}
 	var (
-		prefix = "auth.external.providers." + set.Handle + "."
+		prefix = "auth.external.providers." + eap.Handle + "."
 		pairs  = map[string]interface{}{
-			"enabled":  set.Enabled,
-			"label":    set.Label,
-			"key":      set.Key,
-			"secret":   set.Secret,
-			"scope":    set.Scope,
-			"issuer":   set.IssuerUrl,
-			"redirect": set.RedirectUrl,
-			"weight":   set.Weight,
+			"enabled":  eap.Enabled,
+			"label":    eap.Label,
+			"key":      eap.Key,
+			"secret":   eap.Secret,
+			"scope":    eap.Scope,
+			"issuer":   eap.IssuerUrl,
+			"redirect": eap.RedirectUrl,
+			"weight":   eap.Weight,
 		}
 	)
 
@@ -435,3 +454,12 @@ func (set ExternalAuthProvider) EncodeKV() (vv SettingValueSet, err error) {
 
 	return
 }
+
+// returns true if all relevant props are empty
+func (eap ExternalAuthProvider) empty() bool {
+	return len(
+		eap.RedirectUrl+
+			eap.Secret+
+			eap.Key,
+	) == 0
+}

system/types/app_settings_test.go

@@ -1,7 +1,6 @@
 package types
 
 import (
-	"sort"
 	"testing"
 
 	sqlTypes "github.com/jmoiron/sqlx/types"
@@ -33,27 +32,30 @@ func Test_settingsExtAuthProvidersDecode(t *testing.T) {
 	)
 
 	var (
-		aux = Dst{}
-		kv  = SettingsKV{
-			"providers.foo.enabled":                sqlTypes.JSONText(`true`),
-			"providers.openid-connect.bar.enabled": sqlTypes.JSONText(`true`),
-			"providers.openid-connect.bar.key":     sqlTypes.JSONText(`"K3Y"`),
-			"providers.google.enabled":             sqlTypes.JSONText(`true`),
-			"providers.google.key":                 sqlTypes.JSONText(`"g00gl3"`),
-		}
-
-		eq = Dst{
+		aux = Dst{
 			Providers: ExternalAuthProviderSet{
 				{Handle: "github"},
 				{Handle: "facebook"},
 				{Enabled: true, Key: "g00gl3", Handle: "google"},
 				{Handle: "linkedin"},
-				{Enabled: true, Key: "K3Y", Handle: "openid-connect.bar"},
+				{Enabled: true, Key: "K3Y", Handle: "openid-connect.remove"},
 			},
 		}
-	)
+		kv = SettingsKV{
+			"providers.foo.enabled":                sqlTypes.JSONText(`true`),
+			"providers.openid-connect.bar.enabled": sqlTypes.JSONText(`true`),
+			"providers.openid-connect.bar.key":     sqlTypes.JSONText(`"K3Y"`),
+			"providers.google.enabled":             sqlTypes.JSONText(`true`),
+			"providers.google.key":                 sqlTypes.JSONText(`"g00gl3"`),
 
-	sort.Sort(eq.Providers)
+			// Values with null should not be added!
+			"providers.openid-connect.null.handle": sqlTypes.JSONText(`null`),
+
+			// Values with null should not be added!
+			"providers.openid-connect.remove.handle": sqlTypes.JSONText(`null`),
+			"providers.openid-connect.remove.key":    sqlTypes.JSONText(`null`),
+		}
+	)
 
 	require.NoError(t, DecodeKV(kv, &aux))
 	require.Len(t, aux.Providers, 5)

system/types/kv_decoder.go

@@ -144,16 +144,23 @@ func DecodeKV(kv SettingsKV, dst interface{}, pp ...string) (err error) {
 
 		// Native type
 		if val, ok := kv[key]; ok {
-			// Always use pointer to value
-			if val == nil {
+			// Take care of null values
+			if val == nil || val.String() == "null" {
 				switch structFType.Type.Kind() {
 				case reflect.String:
 					structField.SetString("")
-					continue
+				case reflect.Int, reflect.Int32, reflect.Int64:
+					structField.SetInt(0)
+				case reflect.Uint, reflect.Uint32, reflect.Uint64:
+					structField.SetUint(0)
+				case reflect.Float32, reflect.Float64:
+					structField.SetFloat(0)
 				}
+
+				continue
 			}
 
-			if val.Unmarshal(structField.Addr().Interface()) != nil {
+			if err = val.Unmarshal(structField.Addr().Interface()); err != nil {
 				// Try to get numbers encoded as strings...
 				var tmp interface{}
 				if val.Unmarshal(&tmp) != nil {