diff --git a/system/types/app_settings.go b/system/types/app_settings.go index 983abc9c6..d26241a5d 100644 --- a/system/types/app_settings.go +++ b/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 +} diff --git a/system/types/app_settings_test.go b/system/types/app_settings_test.go index b6a11a3b2..57bbceab7 100644 --- a/system/types/app_settings_test.go +++ b/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) diff --git a/system/types/kv_decoder.go b/system/types/kv_decoder.go index ba89ecc1d..0b9ed9057 100644 --- a/system/types/kv_decoder.go +++ b/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 {