From 7f629df4d56b96adcb64673cf6a19ce2c4426d66 Mon Sep 17 00:00:00 2001 From: Aditya Manthramurthy Date: Fri, 17 Jun 2022 11:39:21 -0700 Subject: [PATCH] Add generic function to retrieve config value with metadata (#15083) `config.ResolveConfigParam` returns the value of a configuration for any subsystem based on checking env, config store, and default value. Also returns info about which config source returned the value. This is useful to return info about config params overridden via env in the user APIs. Currently implemented only for OpenID subsystem, but will be extended for others subsequently. --- cmd/config-current.go | 4 +- internal/config/config.go | 194 ++++++++++++++++++ internal/config/identity/openid/jwt_test.go | 2 +- internal/config/identity/openid/openid.go | 89 +++----- .../config/identity/openid/providercfg.go | 28 +-- 5 files changed, 237 insertions(+), 80 deletions(-) diff --git a/cmd/config-current.go b/cmd/config-current.go index b667b1a8a..7b65a5f53 100644 --- a/cmd/config-current.go +++ b/cmd/config-current.go @@ -338,7 +338,7 @@ func validateSubSysConfig(s config.Config, subSys string, objAPI ObjectLayer) er etcdClnt.Close() } case config.IdentityOpenIDSubSys: - if _, err := openid.LookupConfig(s[config.IdentityOpenIDSubSys], + if _, err := openid.LookupConfig(s, NewGatewayHTTPTransport(), xhttp.DrainBody, globalSite.Region); err != nil { return err } @@ -560,7 +560,7 @@ func lookupConfigs(s config.Config, objAPI ObjectLayer) { logger.LogIf(ctx, fmt.Errorf("CRITICAL: enabling %s is not recommended in a production environment", xtls.EnvIdentityTLSSkipVerify)) } - globalOpenIDConfig, err = openid.LookupConfig(s[config.IdentityOpenIDSubSys], + globalOpenIDConfig, err = openid.LookupConfig(s, NewGatewayHTTPTransport(), xhttp.DrainBody, globalSite.Region) if err != nil { logger.LogIf(ctx, fmt.Errorf("Unable to initialize OpenID: %w", err)) diff --git a/internal/config/config.go b/internal/config/config.go index 3ca9dd03d..441d48dec 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -940,3 +940,197 @@ func (c Config) SetKVS(s string, defaultKVS map[string]KVS) (dynamic bool, err e c[subSys][tgt] = currKVS return dynamic, nil } + +// CheckValidKeys - checks if the config parameters for the given subsystem and +// target are valid. It checks both the configuration store as well as +// environment variables. +func (c Config) CheckValidKeys(subSys string, deprecatedKeys []string) error { + defKVS, ok := DefaultKVS[subSys] + if !ok { + return fmt.Errorf("Subsystem %s does not exist", subSys) + } + + // Make a list of valid keys for the subsystem including the `comment` + // key. + validKeys := make([]string, 0, len(defKVS)+1) + for _, param := range defKVS { + validKeys = append(validKeys, param.Key) + } + validKeys = append(validKeys, Comment) + + subSysEnvVars := env.List(fmt.Sprintf("%s%s", EnvPrefix, strings.ToUpper(subSys))) + + // Set of env vars for the sub-system to validate. + candidates := set.CreateStringSet(subSysEnvVars...) + + // Remove all default target env vars from the candidates set (as they + // are valid). + for _, param := range validKeys { + paramEnvName := getEnvVarName(subSys, Default, param) + candidates.Remove(paramEnvName) + } + + isSingleTarget := SubSystemsSingleTargets.Contains(subSys) + if isSingleTarget && len(candidates) > 0 { + return fmt.Errorf("The following environment variables are unknown: %s", + strings.Join(candidates.ToSlice(), ", ")) + } + + if !isSingleTarget { + // Validate other env vars for all targets. + envVars := candidates.ToSlice() + for _, envVar := range envVars { + for _, param := range validKeys { + pEnvName := getEnvVarName(subSys, Default, param) + Default + if len(envVar) > len(pEnvName) && strings.HasPrefix(envVar, pEnvName) { + // This envVar is valid - it has a + // non-empty target. + candidates.Remove(envVar) + } + } + } + + // Whatever remains are invalid env vars - return an error. + if len(candidates) > 0 { + return fmt.Errorf("The following environment variables are unknown: %s", + strings.Join(candidates.ToSlice(), ", ")) + } + } + + validKeysSet := set.CreateStringSet(validKeys...) + validKeysSet = validKeysSet.Difference(set.CreateStringSet(deprecatedKeys...)) + kvsMap := c[subSys] + for tgt, kvs := range kvsMap { + invalidKV := KVS{} + for _, kv := range kvs { + if !validKeysSet.Contains(kv.Key) { + invalidKV = append(invalidKV, kv) + } + } + if len(invalidKV) > 0 { + return Errorf( + "found invalid keys (%s) for '%s:%s' sub-system, use 'mc admin config reset myminio %s:%s' to fix invalid keys", + invalidKV.String(), subSys, tgt, subSys, tgt) + } + } + return nil +} + +// GetAvailableTargets - returns a list of targets configured for the given +// subsystem (whether they are enabled or not). A target could be configured via +// environment variables or via the configuration store. The default target is +// `_` and is always returned. +func (c Config) GetAvailableTargets(subSys string) ([]string, error) { + if SubSystemsSingleTargets.Contains(subSys) { + return []string{Default}, nil + } + + defKVS, ok := DefaultKVS[subSys] + if !ok { + return nil, fmt.Errorf("Subsystem %s does not exist", subSys) + } + + kvsMap := c[subSys] + s := set.NewStringSet() + + // Add all targets that are configured in the config store. + for k := range kvsMap { + s.Add(k) + } + + // Add targets that are configured via environment variables. + for _, param := range defKVS { + envVarPrefix := getEnvVarName(subSys, Default, param.Key) + Default + envsWithPrefix := env.List(envVarPrefix) + for _, k := range envsWithPrefix { + tgtName := strings.TrimPrefix(k, envVarPrefix) + if tgtName != "" { + s.Add(tgtName) + } + } + } + + return s.ToSlice(), nil +} + +func getEnvVarName(subSys, target, param string) string { + if target == Default { + return fmt.Sprintf("%s%s_%s", EnvPrefix, strings.ToUpper(subSys), strings.ToUpper(param)) + } + + return fmt.Sprintf("%s%s_%s_%s", EnvPrefix, strings.ToUpper(subSys), strings.ToUpper(param), target) +} + +var resolvableSubsystems = set.CreateStringSet(IdentityOpenIDSubSys) + +// ValueSource represents the source of a config parameter value. +type ValueSource uint8 + +// Constants for ValueSource +const ( + ValueSourceAbsent ValueSource = iota // this is an error case + ValueSourceDef + ValueSourceCfg + ValueSourceEnv +) + +// ResolveConfigParam returns the effective value of a configuration parameter, +// within a subsystem and subsystem target. The effective value is, in order of +// decreasing precedence: +// +// 1. the value of the corresponding environment variable if set, +// 2. the value of the parameter in the config store if set, +// 3. the default value, +// +// This function only works for a subset of sub-systems, others return +// `ValueSourceAbsent`. FIXME: some parameters have custom environment +// variables for which support needs to be added. +func (c Config) ResolveConfigParam(subSys, target, cfgParam string) (value string, cs ValueSource) { + // cs = ValueSourceAbsent initially as it is iota by default. + + // Initially only support OpenID + if !resolvableSubsystems.Contains(subSys) { + return + } + + // Check if config param requested is valid. + defKVS, ok := DefaultKVS[subSys] + if !ok { + return + } + + defValue, isFound := defKVS.Lookup(cfgParam) + if !isFound { + return + } + + if target == "" { + target = Default + } + + envVar := getEnvVarName(subSys, target, cfgParam) + + // Lookup Env var. + value = env.Get(envVar, "") + if value != "" { + cs = ValueSourceEnv + return + } + + // Lookup config store. + if subSysStore, ok := c[subSys]; ok { + if kvs, ok2 := subSysStore[target]; ok2 { + var ok3 bool + value, ok3 = kvs.Lookup(cfgParam) + if ok3 { + cs = ValueSourceCfg + return + } + } + } + + // Return the default value. + value = defValue + cs = ValueSourceDef + return +} diff --git a/internal/config/identity/openid/jwt_test.go b/internal/config/identity/openid/jwt_test.go index d270548bb..44425ff11 100644 --- a/internal/config/identity/openid/jwt_test.go +++ b/internal/config/identity/openid/jwt_test.go @@ -243,7 +243,7 @@ func TestKeycloakProviderInitialization(t *testing.T) { testKvs.Set(Vendor, "keycloak") testKvs.Set(KeyCloakRealm, "TestRealm") testKvs.Set(KeyCloakAdminURL, "http://keycloak.test/auth/admin") - cfgGet := func(env, param string) string { + cfgGet := func(param string) string { return testKvs.Get(param) } diff --git a/internal/config/identity/openid/openid.go b/internal/config/identity/openid/openid.go index e9f800f0a..d9878d473 100644 --- a/internal/config/identity/openid/openid.go +++ b/internal/config/identity/openid/openid.go @@ -36,49 +36,32 @@ import ( "github.com/minio/minio/internal/config" "github.com/minio/minio/internal/config/identity/openid/provider" "github.com/minio/minio/internal/hash/sha256" - "github.com/minio/pkg/env" iampolicy "github.com/minio/pkg/iam/policy" xnet "github.com/minio/pkg/net" ) // OpenID keys and envs. const ( - JwksURL = "jwks_url" + ClientID = "client_id" + ClientSecret = "client_secret" ConfigURL = "config_url" ClaimName = "claim_name" ClaimUserinfo = "claim_userinfo" - ClaimPrefix = "claim_prefix" - ClientID = "client_id" - ClientSecret = "client_secret" RolePolicy = "role_policy" DisplayName = "display_name" - Vendor = "vendor" Scopes = "scopes" RedirectURI = "redirect_uri" RedirectURIDynamic = "redirect_uri_dynamic" + Vendor = "vendor" // Vendor specific ENV only enabled if the Vendor matches == "vendor" KeyCloakRealm = "keycloak_realm" KeyCloakAdminURL = "keycloak_admin_url" - EnvIdentityOpenIDEnable = "MINIO_IDENTITY_OPENID_ENABLE" - EnvIdentityOpenIDVendor = "MINIO_IDENTITY_OPENID_VENDOR" - EnvIdentityOpenIDClientID = "MINIO_IDENTITY_OPENID_CLIENT_ID" - EnvIdentityOpenIDClientSecret = "MINIO_IDENTITY_OPENID_CLIENT_SECRET" - EnvIdentityOpenIDURL = "MINIO_IDENTITY_OPENID_CONFIG_URL" - EnvIdentityOpenIDClaimName = "MINIO_IDENTITY_OPENID_CLAIM_NAME" - EnvIdentityOpenIDClaimUserInfo = "MINIO_IDENTITY_OPENID_CLAIM_USERINFO" - EnvIdentityOpenIDClaimPrefix = "MINIO_IDENTITY_OPENID_CLAIM_PREFIX" - EnvIdentityOpenIDRolePolicy = "MINIO_IDENTITY_OPENID_ROLE_POLICY" - EnvIdentityOpenIDRedirectURI = "MINIO_IDENTITY_OPENID_REDIRECT_URI" - EnvIdentityOpenIDRedirectURIDynamic = "MINIO_IDENTITY_OPENID_REDIRECT_URI_DYNAMIC" - EnvIdentityOpenIDScopes = "MINIO_IDENTITY_OPENID_SCOPES" - EnvIdentityOpenIDDisplayName = "MINIO_IDENTITY_OPENID_DISPLAY_NAME" - - // Vendor specific ENVs only enabled if the Vendor matches == "vendor" - EnvIdentityOpenIDKeyCloakRealm = "MINIO_IDENTITY_OPENID_KEYCLOAK_REALM" - EnvIdentityOpenIDKeyCloakAdminURL = "MINIO_IDENTITY_OPENID_KEYCLOAK_ADMIN_URL" + // Removed params + JwksURL = "jwks_url" + ClaimPrefix = "claim_prefix" ) // DefaultKVS - default config for OpenID config @@ -191,7 +174,7 @@ func (r *Config) Clone() Config { } // LookupConfig lookup jwks from config, override with any ENVs. -func LookupConfig(kvsMap map[string]config.KVS, transport http.RoundTripper, closeRespFn func(io.ReadCloser), serverRegion string) (c Config, err error) { +func LookupConfig(s config.Config, transport http.RoundTripper, closeRespFn func(io.ReadCloser), serverRegion string) (c Config, err error) { openIDClientTransport := http.DefaultTransport if transport != nil { openIDClientTransport = transport @@ -209,48 +192,28 @@ func LookupConfig(kvsMap map[string]config.KVS, transport http.RoundTripper, clo closeRespFn: closeRespFn, } - // Make a copy of the config we received so we can mutate it safely. - kvsMap2 := make(map[string]config.KVS, len(kvsMap)) - for k, v := range kvsMap { - kvsMap2[k] = v - } - - // Add in each configuration name found from environment variables, i.e. - // if we see MINIO_IDENTITY_OPENID_CONFIG_URL_2, we add the key "2" to - // `kvsMap2` if it does not already exist. - envs := env.List(EnvIdentityOpenIDURL + config.Default) - for _, k := range envs { - cfgName := strings.TrimPrefix(k, EnvIdentityOpenIDURL+config.Default) - if cfgName == "" { - return c, config.Errorf("Environment variable must have a non-empty config name: %s", k) - } - - // It is possible that some variables were specified via config - // commands and some variables are intended to be overridden - // from the environment, so we ensure that the key is not - // overwritten in `kvsMap2` as it may have existing config. - if _, ok := kvsMap2[cfgName]; !ok { - kvsMap2[cfgName] = DefaultKVS - } - } - var ( hasLegacyPolicyMapping = false seenClientIDs = set.NewStringSet() ) - for cfgName, kvs := range kvsMap2 { - // remove this since we have removed support for this already. - kvs.Delete(JwksURL) - if err = config.CheckValidKeys(config.IdentityOpenIDSubSys, kvs, DefaultKVS); err != nil { - return c, err - } + // remove this since we have removed support for this already. + deprecatedKeys := []string{JwksURL} + if err := s.CheckValidKeys(config.IdentityOpenIDSubSys, deprecatedKeys); err != nil { + return c, err + } - getCfgVal := func(envVar, cfgParam string) string { - if cfgName != config.Default { - envVar += config.Default + cfgName - } - return env.Get(envVar, kvs.Get(cfgParam)) + openIDTargets, err := s.GetAvailableTargets(config.IdentityOpenIDSubSys) + if err != nil { + return c, err + } + + for _, cfgName := range openIDTargets { + getCfgVal := func(cfgParam string) string { + // As parameters are already validated, we skip checking + // if the config param was found. + val, _ := s.ResolveConfigParam(config.IdentityOpenIDSubSys, cfgName, cfgParam) + return val } // In the past, when only one openID provider was allowed, there @@ -261,7 +224,7 @@ func LookupConfig(kvsMap map[string]config.KVS, transport http.RoundTripper, clo // setting, otherwise we treat it as enabled if some important // parameters are non-empty. var ( - cfgEnableVal = getCfgVal(EnvIdentityOpenIDEnable, config.Enable) + cfgEnableVal = getCfgVal(config.Enable) isExplicitlyEnabled = false ) if cfgEnableVal != "" { @@ -281,7 +244,7 @@ func LookupConfig(kvsMap map[string]config.KVS, transport http.RoundTripper, clo } p := newProviderCfgFromConfig(getCfgVal) - configURL := getCfgVal(EnvIdentityOpenIDURL, ConfigURL) + configURL := getCfgVal(ConfigURL) if !isExplicitlyEnabled { enabled = true @@ -315,7 +278,7 @@ func LookupConfig(kvsMap map[string]config.KVS, transport http.RoundTripper, clo return c, errors.New("please specify config_url to enable fetching claims from UserInfo endpoint") } - if scopeList := getCfgVal(EnvIdentityOpenIDScopes, Scopes); scopeList != "" { + if scopeList := getCfgVal(Scopes); scopeList != "" { var scopes []string for _, scope := range strings.Split(scopeList, ",") { scope = strings.TrimSpace(scope) diff --git a/internal/config/identity/openid/providercfg.go b/internal/config/identity/openid/providercfg.go index 4981a520f..c7d871f4b 100644 --- a/internal/config/identity/openid/providercfg.go +++ b/internal/config/identity/openid/providercfg.go @@ -52,17 +52,17 @@ type providerCfg struct { provider provider.Provider } -func newProviderCfgFromConfig(getCfgVal func(env, cfgName string) string) providerCfg { +func newProviderCfgFromConfig(getCfgVal func(cfgName string) string) providerCfg { return providerCfg{ - DisplayName: getCfgVal(EnvIdentityOpenIDDisplayName, DisplayName), - ClaimName: getCfgVal(EnvIdentityOpenIDClaimName, ClaimName), - ClaimUserinfo: getCfgVal(EnvIdentityOpenIDClaimUserInfo, ClaimUserinfo) == config.EnableOn, - ClaimPrefix: getCfgVal(EnvIdentityOpenIDClaimPrefix, ClaimPrefix), - RedirectURI: getCfgVal(EnvIdentityOpenIDRedirectURI, RedirectURI), - RedirectURIDynamic: getCfgVal(EnvIdentityOpenIDRedirectURIDynamic, RedirectURIDynamic) == config.EnableOn, - ClientID: getCfgVal(EnvIdentityOpenIDClientID, ClientID), - ClientSecret: getCfgVal(EnvIdentityOpenIDClientSecret, ClientSecret), - RolePolicy: getCfgVal(EnvIdentityOpenIDRolePolicy, RolePolicy), + DisplayName: getCfgVal(DisplayName), + ClaimName: getCfgVal(ClaimName), + ClaimUserinfo: getCfgVal(ClaimUserinfo) == config.EnableOn, + ClaimPrefix: getCfgVal(ClaimPrefix), + RedirectURI: getCfgVal(RedirectURI), + RedirectURIDynamic: getCfgVal(RedirectURIDynamic) == config.EnableOn, + ClientID: getCfgVal(ClientID), + ClientSecret: getCfgVal(ClientSecret), + RolePolicy: getCfgVal(RolePolicy), } } @@ -72,16 +72,16 @@ const ( // initializeProvider initializes if any additional vendor specific information // was provided, initialization will return an error initial login fails. -func (p *providerCfg) initializeProvider(cfgGet func(string, string) string, transport http.RoundTripper) error { - vendor := cfgGet(EnvIdentityOpenIDVendor, Vendor) +func (p *providerCfg) initializeProvider(cfgGet func(string) string, transport http.RoundTripper) error { + vendor := cfgGet(Vendor) if vendor == "" { return nil } var err error switch vendor { case keyCloakVendor: - adminURL := cfgGet(EnvIdentityOpenIDKeyCloakAdminURL, KeyCloakAdminURL) - realm := cfgGet(EnvIdentityOpenIDKeyCloakRealm, KeyCloakRealm) + adminURL := cfgGet(KeyCloakAdminURL) + realm := cfgGet(KeyCloakRealm) p.provider, err = provider.KeyCloak( provider.WithAdminURL(adminURL), provider.WithOpenIDConfig(provider.DiscoveryDoc(p.DiscoveryDoc)),