From d372f56f8df05586a7f64bc809a840f64c6d9be4 Mon Sep 17 00:00:00 2001 From: Lenin Alevski Date: Thu, 7 Apr 2022 14:40:44 -0700 Subject: [PATCH] fix: ParseRawConfiguration function was incorrectly parsing = symbols - added unit tests - uses similar logic to the one used on MinIO Signed-off-by: Lenin Alevski --- .../tenant-external-idp-oidc/tenant.yaml | 2 + pkg/apis/minio.min.io/v2/helper.go | 83 ++++++++++++---- pkg/apis/minio.min.io/v2/helper_test.go | 96 +++++++++++++++++++ 3 files changed, 165 insertions(+), 16 deletions(-) diff --git a/examples/kustomization/tenant-external-idp-oidc/tenant.yaml b/examples/kustomization/tenant-external-idp-oidc/tenant.yaml index 0b9232b4f5e..dabd1bf2601 100644 --- a/examples/kustomization/tenant-external-idp-oidc/tenant.yaml +++ b/examples/kustomization/tenant-external-idp-oidc/tenant.yaml @@ -16,3 +16,5 @@ spec: value: "openid,profile,email" - name: MINIO_IDENTITY_OPENID_CLAIM_NAME value: "https://min.io/policy" + - name: MINIO_IDENTITY_OPENID_REDIRECT_URI + value: "https://your-console-endpoint.com/oauth_callback" diff --git a/pkg/apis/minio.min.io/v2/helper.go b/pkg/apis/minio.min.io/v2/helper.go index 4c1ef80d694..add50c5da4b 100644 --- a/pkg/apis/minio.min.io/v2/helper.go +++ b/pkg/apis/minio.min.io/v2/helper.go @@ -979,29 +979,80 @@ func (t *Tenant) GetTenantServiceURL() (svcURL string) { return fmt.Sprintf("%s://%s", scheme, svc) } +type envKV struct { + Key string + Value string + Skip bool +} + +func (e envKV) String() string { + if e.Skip { + return "" + } + return fmt.Sprintf("%s=%s", e.Key, e.Value) +} + +func parsEnvEntry(envEntry string) (envKV, error) { + envEntry = strings.TrimSpace(envEntry) + if envEntry == "" { + // Skip all empty lines + return envKV{ + Skip: true, + }, nil + } + const envSeparator = "=" + envTokens := strings.SplitN(strings.TrimSpace(strings.TrimPrefix(envEntry, "export")), envSeparator, 2) + if len(envTokens) != 2 { + return envKV{}, fmt.Errorf("envEntry malformed; %s, expected to be of form 'KEY=value'", envEntry) + } + key := envTokens[0] + val := envTokens[1] + + if strings.HasPrefix(key, "#") { + // Skip commented lines + return envKV{ + Skip: true, + }, nil + } + + // Remove quotes from the value if found + if len(val) >= 2 { + quote := val[0] + if (quote == '"' || quote == '\'') && val[len(val)-1] == quote { + val = val[1 : len(val)-1] + } + } + return envKV{ + Key: key, + Value: val, + }, nil +} + // ParseRawConfiguration map[string][]byte representation of the MinIO config.env file func ParseRawConfiguration(configuration []byte) (config map[string][]byte) { config = map[string][]byte{} if configuration != nil { scanner := bufio.NewScanner(strings.NewReader(string(configuration))) for scanner.Scan() { - line := scanner.Text() - // parse only exported environment variables and ignore everything else - if strings.HasPrefix(line, "export") { - envVar := strings.Split(line, "=") - if len(envVar) == 2 { - // extract env variable key - confKey := strings.TrimSpace(strings.TrimPrefix(envVar[0], "export")) - // remove first and last quotes from value and trim spaces - confValue := strings.TrimSpace(strings.TrimSuffix(strings.TrimPrefix(envVar[1], "\""), "\"")) - config[confKey] = []byte(confValue) - if confKey == "MINIO_ROOT_USER" || confKey == "MINIO_ACCESS_KEY" { - config["accesskey"] = config[confKey] - } else if confKey == "MINIO_ROOT_PASSWORD" || confKey == "MINIO_SECRET_KEY" { - config["secretkey"] = config[confKey] - } - } + ekv, err := parsEnvEntry(scanner.Text()) + if err != nil { + klog.Errorf("Error parsing tenant configuration: %v", err.Error()) + continue + } + if ekv.Skip { + // Skips empty lines + continue } + config[ekv.Key] = []byte(ekv.Value) + if ekv.Key == "MINIO_ROOT_USER" || ekv.Key == "MINIO_ACCESS_KEY" { + config["accesskey"] = config[ekv.Key] + } else if ekv.Key == "MINIO_ROOT_PASSWORD" || ekv.Key == "MINIO_SECRET_KEY" { + config["secretkey"] = config[ekv.Key] + } + } + if err := scanner.Err(); err != nil { + klog.Errorf("Error parsing tenant configuration: %v", err.Error()) + return config } } return config diff --git a/pkg/apis/minio.min.io/v2/helper_test.go b/pkg/apis/minio.min.io/v2/helper_test.go index 487106ed396..92f3a7f2811 100644 --- a/pkg/apis/minio.min.io/v2/helper_test.go +++ b/pkg/apis/minio.min.io/v2/helper_test.go @@ -221,3 +221,99 @@ func TestCompareEnvs(t *testing.T) { }) } } + +func TestParseRawConfiguration(t *testing.T) { + type args struct { + configuration []byte + } + tests := []struct { + name string + args args + wantConfig map[string][]byte + }{ + { + name: "ldap-configuration", + args: args{ + configuration: []byte(` +export MINIO_ROOT_USER=minio +export MINIO_ROOT_PASSWORD=minio123 + +export MINIO_IDENTITY_LDAP_SERVER_ADDR=localhost:389 +export MINIO_IDENTITY_LDAP_LOOKUP_BIND_DN="cn=admin,dc=min,dc=io" +export MINIO_IDENTITY_LDAP_LOOKUP_BIND_PASSWORD="admin" +export MINIO_IDENTITY_LDAP_USER_DN_SEARCH_BASE_DN="dc=min,dc=io" +export MINIO_IDENTITY_LDAP_USER_DN_SEARCH_FILTER="(uid=%s)" +export MINIO_IDENTITY_LDAP_GROUP_SEARCH_BASE_DN="ou=swengg,dc=min,dc=io" +export MINIO_IDENTITY_LDAP_GROUP_SEARCH_FILTER="(&(objectclass=groupOfNames)(member=%d))" +export MINIO_IDENTITY_LDAP_SERVER_INSECURE="on" + +export MINIO_BROWSER_REDIRECT_URL=http://localhost:9001 +export MINIO_SERVER_URL=http://localhost:9000`), + }, + wantConfig: map[string][]byte{ + "accesskey": []byte("minio"), + "secretkey": []byte("minio123"), + "MINIO_ROOT_USER": []byte("minio"), + "MINIO_ROOT_PASSWORD": []byte("minio123"), + "MINIO_IDENTITY_LDAP_SERVER_ADDR": []byte("localhost:389"), + "MINIO_IDENTITY_LDAP_LOOKUP_BIND_DN": []byte("cn=admin,dc=min,dc=io"), + "MINIO_IDENTITY_LDAP_LOOKUP_BIND_PASSWORD": []byte("admin"), + "MINIO_IDENTITY_LDAP_USER_DN_SEARCH_BASE_DN": []byte("dc=min,dc=io"), + "MINIO_IDENTITY_LDAP_USER_DN_SEARCH_FILTER": []byte("(uid=%s)"), + "MINIO_IDENTITY_LDAP_GROUP_SEARCH_BASE_DN": []byte("ou=swengg,dc=min,dc=io"), + "MINIO_IDENTITY_LDAP_GROUP_SEARCH_FILTER": []byte("(&(objectclass=groupOfNames)(member=%d))"), + "MINIO_IDENTITY_LDAP_SERVER_INSECURE": []byte("on"), + "MINIO_BROWSER_REDIRECT_URL": []byte("http://localhost:9001"), + "MINIO_SERVER_URL": []byte("http://localhost:9000"), + }, + }, + { + name: "oidc-configuration", + args: args{ + configuration: []byte(` +#!/bin/bash + +# // Auth0 Rule +# function (user, context, callback) { +# const namespace = 'https://min.io/'; +# context.accessToken[namespace + 'policy'] = 'mcsAdmin'; +# context.idToken[namespace + 'policy'] = 'mcsAdmin'; +# callback(null, user, context); +# } + +export MINIO_ROOT_USER=minio +export MINIO_ROOT_PASSWORD=minio123 +export MINIO_IDENTITY_OPENID_CONFIG_URL=https://*******************/.well-known/openid-configuration +export MINIO_IDENTITY_OPENID_CLIENT_ID="****************************" +export MINIO_IDENTITY_OPENID_CLIENT_SECRET="********************" +export MINIO_IDENTITY_OPENID_SCOPES="openid,profile,email" +export MINIO_IDENTITY_OPENID_CLAIM_NAME="https://min.io/policy" +export MINIO_BROWSER_REDIRECT_URL=http://localhost:9001 +export MINIO_SERVER_URL=http://localhost:9000 +./minio server ~/Data --console-address ":9001"`), + }, + wantConfig: map[string][]byte{ + "accesskey": []byte("minio"), + "secretkey": []byte("minio123"), + "MINIO_ROOT_USER": []byte("minio"), + "MINIO_ROOT_PASSWORD": []byte("minio123"), + "MINIO_IDENTITY_OPENID_CONFIG_URL": []byte("https://*******************/.well-known/openid-configuration"), + "MINIO_IDENTITY_OPENID_CLIENT_ID": []byte("****************************"), + "MINIO_IDENTITY_OPENID_CLIENT_SECRET": []byte("********************"), + "MINIO_IDENTITY_OPENID_SCOPES": []byte("openid,profile,email"), + "MINIO_IDENTITY_OPENID_CLAIM_NAME": []byte("https://min.io/policy"), + "MINIO_BROWSER_REDIRECT_URL": []byte("http://localhost:9001"), + "MINIO_SERVER_URL": []byte("http://localhost:9000"), + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + parsedValues := ParseRawConfiguration(tt.args.configuration) + assert.Equalf(t, len(tt.wantConfig), len(parsedValues), "ParseRawConfiguration(%v)", string(tt.args.configuration)) + for k := range tt.wantConfig { + assert.Equalf(t, string(tt.wantConfig[k]), string(parsedValues[k]), "ParseRawConfiguration(%v)", string(tt.args.configuration)) + } + }) + } +}