Allow to disable os.ExpandEnv for storage + connector configs by env variable DEX_EXPAND_ENV = false

Signed-off-by: Martin Heide <martin.heide@faro.com>
This commit is contained in:
Martin Heide 2020-12-29 12:54:02 +00:00
parent bae6bbb171
commit 4cb5577e11
3 changed files with 80 additions and 14 deletions

View file

@ -57,7 +57,6 @@ jobs:
- name: Run tests - name: Run tests
run: make testall run: make testall
env: env:
DEX_FOO_USER_PASSWORD: $2a$10$33EMT0cVYVlPy6WAMCLsceLYjWhuHpbz5yuZxu/GAFj03J9Lytjuy
DEX_MYSQL_DATABASE: dex DEX_MYSQL_DATABASE: dex
DEX_MYSQL_USER: root DEX_MYSQL_USER: root
DEX_MYSQL_PASSWORD: root DEX_MYSQL_PASSWORD: root

View file

@ -5,6 +5,7 @@ import (
"encoding/json" "encoding/json"
"fmt" "fmt"
"os" "os"
"strconv"
"strings" "strings"
"golang.org/x/crypto/bcrypt" "golang.org/x/crypto/bcrypt"
@ -181,6 +182,19 @@ var storages = map[string]func() StorageConfig{
"mysql": func() StorageConfig { return new(sql.MySQL) }, "mysql": func() StorageConfig { return new(sql.MySQL) },
} }
// isExpandEnvEnabled returns if os.ExpandEnv should be used for each storage and connector config.
// Disabling this feature avoids surprises e.g. if the LDAP bind password contains a dollar character.
// Returns false if the env variable "DEX_EXPAND_ENV" is a falsy string, e.g. "false".
// Returns true if the env variable is unset or a truthy string, e.g. "true", or can't be parsed as bool.
func isExpandEnvEnabled() bool {
enabled, err := strconv.ParseBool(os.Getenv("DEX_EXPAND_ENV"))
if err != nil {
// Unset, empty string or can't be parsed as bool: Default = true.
return true
}
return enabled
}
// UnmarshalJSON allows Storage to implement the unmarshaler interface to // UnmarshalJSON allows Storage to implement the unmarshaler interface to
// dynamically determine the type of the storage config. // dynamically determine the type of the storage config.
func (s *Storage) UnmarshalJSON(b []byte) error { func (s *Storage) UnmarshalJSON(b []byte) error {
@ -198,7 +212,11 @@ func (s *Storage) UnmarshalJSON(b []byte) error {
storageConfig := f() storageConfig := f()
if len(store.Config) != 0 { if len(store.Config) != 0 {
data := []byte(os.ExpandEnv(string(store.Config))) data := []byte(store.Config)
if isExpandEnvEnabled() {
// Caution, we're expanding in the raw JSON/YAML source. This may not be what the admin expects.
data = []byte(os.ExpandEnv(string(store.Config)))
}
if err := json.Unmarshal(data, storageConfig); err != nil { if err := json.Unmarshal(data, storageConfig); err != nil {
return fmt.Errorf("parse storage config: %v", err) return fmt.Errorf("parse storage config: %v", err)
} }
@ -240,7 +258,11 @@ func (c *Connector) UnmarshalJSON(b []byte) error {
connConfig := f() connConfig := f()
if len(conn.Config) != 0 { if len(conn.Config) != 0 {
data := []byte(os.ExpandEnv(string(conn.Config))) data := []byte(conn.Config)
if isExpandEnvEnabled() {
// Caution, we're expanding in the raw JSON/YAML source. This may not be what the admin expects.
data = []byte(os.ExpandEnv(string(conn.Config)))
}
if err := json.Unmarshal(data, connConfig); err != nil { if err := json.Unmarshal(data, connConfig); err != nil {
return fmt.Errorf("parse connector config: %v", err) return fmt.Errorf("parse connector config: %v", err)
} }

View file

@ -16,8 +16,6 @@ import (
var _ = yaml.YAMLToJSON var _ = yaml.YAMLToJSON
const testHashStaticPasswordEnv = "DEX_FOO_USER_PASSWORD"
func TestValidConfiguration(t *testing.T) { func TestValidConfiguration(t *testing.T) {
configuration := Config{ configuration := Config{
Issuer: "http://127.0.0.1:5556/dex", Issuer: "http://127.0.0.1:5556/dex",
@ -110,7 +108,7 @@ staticPasswords:
hash: "$2a$10$33EMT0cVYVlPy6WAMCLsceLYjWhuHpbz5yuZxu/GAFj03J9Lytjuy" hash: "$2a$10$33EMT0cVYVlPy6WAMCLsceLYjWhuHpbz5yuZxu/GAFj03J9Lytjuy"
username: "admin" username: "admin"
userID: "08a8684b-db88-4b73-90a9-3cd1661f5466" userID: "08a8684b-db88-4b73-90a9-3cd1661f5466"
- email: "foo@example.com" - email: "foo@example.com"
# base64'd value of the same bcrypt hash above. We want to be able to parse both of these # base64'd value of the same bcrypt hash above. We want to be able to parse both of these
hash: "JDJhJDEwJDMzRU1UMGNWWVZsUHk2V0FNQ0xzY2VMWWpXaHVIcGJ6NXl1Wnh1L0dBRmowM0o5THl0anV5" hash: "JDJhJDEwJDMzRU1UMGNWWVZsUHk2V0FNQ0xzY2VMWWpXaHVIcGJ6NXl1Wnh1L0dBRmowM0o5THl0anV5"
username: "foo" username: "foo"
@ -219,17 +217,54 @@ logger:
} }
} }
func TestUnmarshalConfigWithEnv(t *testing.T) { func TestUnmarshalConfigWithEnvNoExpand(t *testing.T) {
staticPasswordEnv := os.Getenv(testHashStaticPasswordEnv) // If the env variable DEX_EXPAND_ENV is set and has a "falsy" value, os.ExpandEnv is disabled.
if staticPasswordEnv == "" { // ParseBool: "It accepts 1, t, T, TRUE, true, True, 0, f, F, FALSE, false, False."
t.Skipf("test environment variable %q not set, skipping", testHashStaticPasswordEnv) checkUnmarshalConfigWithEnv(t, "0", false)
checkUnmarshalConfigWithEnv(t, "f", false)
checkUnmarshalConfigWithEnv(t, "F", false)
checkUnmarshalConfigWithEnv(t, "FALSE", false)
checkUnmarshalConfigWithEnv(t, "false", false)
checkUnmarshalConfigWithEnv(t, "False", false)
os.Unsetenv("DEX_EXPAND_ENV")
}
func TestUnmarshalConfigWithEnvExpand(t *testing.T) {
// If the env variable DEX_EXPAND_ENV is unset or has a "truthy" or unknown value, os.ExpandEnv is enabled.
// ParseBool: "It accepts 1, t, T, TRUE, true, True, 0, f, F, FALSE, false, False."
checkUnmarshalConfigWithEnv(t, "1", true)
checkUnmarshalConfigWithEnv(t, "t", true)
checkUnmarshalConfigWithEnv(t, "T", true)
checkUnmarshalConfigWithEnv(t, "TRUE", true)
checkUnmarshalConfigWithEnv(t, "true", true)
checkUnmarshalConfigWithEnv(t, "True", true)
// Values that can't be parsed as bool:
checkUnmarshalConfigWithEnv(t, "UNSET", true)
checkUnmarshalConfigWithEnv(t, "", true)
checkUnmarshalConfigWithEnv(t, "whatever - true is default", true)
os.Unsetenv("DEX_EXPAND_ENV")
}
func checkUnmarshalConfigWithEnv(t *testing.T, dexExpandEnv string, wantExpandEnv bool) {
// For hashFromEnv:
os.Setenv("DEX_FOO_USER_PASSWORD", "$2a$10$33EMT0cVYVlPy6WAMCLsceLYjWhuHpbz5yuZxu/GAFj03J9Lytjuy")
// For os.ExpandEnv ($VAR -> value_of_VAR):
os.Setenv("DEX_FOO_POSTGRES_HOST", "10.0.0.1")
os.Setenv("DEX_FOO_OIDC_CLIENT_SECRET", "bar")
if dexExpandEnv != "UNSET" {
os.Setenv("DEX_EXPAND_ENV", dexExpandEnv)
} else {
os.Unsetenv("DEX_EXPAND_ENV")
} }
rawConfig := []byte(` rawConfig := []byte(`
issuer: http://127.0.0.1:5556/dex issuer: http://127.0.0.1:5556/dex
storage: storage:
type: postgres type: postgres
config: config:
host: 10.0.0.1 # Env variables are expanded in raw YAML source.
# Single quotes work fine, as long as the env variable doesn't contain any.
host: '$DEX_FOO_POSTGRES_HOST'
port: 65432 port: 65432
maxOpenConns: 5 maxOpenConns: 5
maxIdleConns: 3 maxIdleConns: 3
@ -263,7 +298,9 @@ connectors:
config: config:
issuer: https://accounts.google.com issuer: https://accounts.google.com
clientID: foo clientID: foo
clientSecret: bar # Env variables are expanded in raw YAML source.
# Single quotes work fine, as long as the env variable doesn't contain any.
clientSecret: '$DEX_FOO_OIDC_CLIENT_SECRET'
redirectURI: http://127.0.0.1:5556/dex/callback/google redirectURI: http://127.0.0.1:5556/dex/callback/google
enablePasswordDB: true enablePasswordDB: true
@ -288,13 +325,21 @@ logger:
format: "json" format: "json"
`) `)
// This is not a valid hostname. It's only used to check whether os.ExpandEnv was applied or not.
wantPostgresHost := "$DEX_FOO_POSTGRES_HOST"
wantOidcClientSecret := "$DEX_FOO_OIDC_CLIENT_SECRET"
if wantExpandEnv {
wantPostgresHost = "10.0.0.1"
wantOidcClientSecret = "bar"
}
want := Config{ want := Config{
Issuer: "http://127.0.0.1:5556/dex", Issuer: "http://127.0.0.1:5556/dex",
Storage: Storage{ Storage: Storage{
Type: "postgres", Type: "postgres",
Config: &sql.Postgres{ Config: &sql.Postgres{
NetworkDB: sql.NetworkDB{ NetworkDB: sql.NetworkDB{
Host: "10.0.0.1", Host: wantPostgresHost,
Port: 65432, Port: 65432,
MaxOpenConns: 5, MaxOpenConns: 5,
MaxIdleConns: 3, MaxIdleConns: 3,
@ -339,7 +384,7 @@ logger:
Config: &oidc.Config{ Config: &oidc.Config{
Issuer: "https://accounts.google.com", Issuer: "https://accounts.google.com",
ClientID: "foo", ClientID: "foo",
ClientSecret: "bar", ClientSecret: wantOidcClientSecret,
RedirectURI: "http://127.0.0.1:5556/dex/callback/google", RedirectURI: "http://127.0.0.1:5556/dex/callback/google",
}, },
}, },