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)),