fix bug in whitelist

yottahmd · yottahmd · commit 019e94307123 · 2026-01-11T20:08:56.000+09:00
diff --git a/internal/common/config/loader.go b/internal/common/config/loader.go
@@ -434,16 +434,17 @@ func (l *ConfigLoader) loadServerConfig(cfg *Config, def Definition) {
 			cfg.Server.Auth.OIDC.ClientSecret = oidc.ClientSecret
 			cfg.Server.Auth.OIDC.ClientUrl = oidc.ClientUrl
 			cfg.Server.Auth.OIDC.Issuer = oidc.Issuer
-			cfg.Server.Auth.OIDC.Scopes = oidc.Scopes
-			cfg.Server.Auth.OIDC.Whitelist = oidc.Whitelist
+			// Use parseStringList to support both comma-separated strings and YAML lists
+			cfg.Server.Auth.OIDC.Scopes = parseStringList(l.v.Get("auth.oidc.scopes"))
+			cfg.Server.Auth.OIDC.Whitelist = parseStringList(l.v.Get("auth.oidc.whitelist"))
 			// Builtin-specific fields (only used when auth.mode=builtin)
 			if oidc.AutoSignup != nil {
 				cfg.Server.Auth.OIDC.AutoSignup = *oidc.AutoSignup
 			} else {
 				// Default to true - if OIDC is configured, auto-signup is expected
 				cfg.Server.Auth.OIDC.AutoSignup = true
 			}
-			cfg.Server.Auth.OIDC.AllowedDomains = oidc.AllowedDomains
+			cfg.Server.Auth.OIDC.AllowedDomains = parseStringList(l.v.Get("auth.oidc.allowedDomains"))
 			cfg.Server.Auth.OIDC.ButtonLabel = oidc.ButtonLabel
 			// Load role mapping configuration
 			if oidc.RoleMapping != nil {
@@ -1246,6 +1247,35 @@ func parseWorkerLabels(labelsStr string) map[string]string {
 	return labels
 }
 
+// parseStringList parses input that can be either a comma-separated string or a list of strings.
+// This allows config values to be specified as either:
+//   - YAML list: ["a", "b", "c"]
+//   - Comma-separated string: "a,b,c"
+//
+// Empty strings and whitespace-only entries are filtered out.
+func parseStringList(input interface{}) []string {
+	var result []string
+	switch v := input.(type) {
+	case string:
+		if v != "" {
+			for _, s := range strings.Split(v, ",") {
+				if trimmed := strings.TrimSpace(s); trimmed != "" {
+					result = append(result, trimmed)
+				}
+			}
+		}
+	case []interface{}:
+		for _, item := range v {
+			if s, ok := item.(string); ok && s != "" {
+				result = append(result, s)
+			}
+		}
+	case []string:
+		result = v
+	}
+	return result
+}
+
 func cleanServerBasePath(s string) string {
 	if s == "" {
 		return ""
diff --git a/internal/service/oidcprovision/service.go b/internal/service/oidcprovision/service.go
@@ -240,13 +240,15 @@ func (s *Service) syncUserRole(ctx context.Context, user *auth.User, claims OIDC
 // Logic:
 //   - If whitelist is not empty and email is in whitelist: ALLOW
 //   - If allowedDomains is not empty and email domain is in allowedDomains: ALLOW
-//   - If allowedDomains is not empty and email domain is NOT in allowedDomains: DENY
+//   - If either whitelist or allowedDomains is configured but email doesn't match: DENY
 //   - If both whitelist and allowedDomains are empty: ALLOW (no restrictions)
 func (s *Service) isEmailAllowed(email string) bool {
 	email = strings.ToLower(email)
+	hasWhitelist := len(s.config.Whitelist) > 0
+	hasAllowedDomains := len(s.config.AllowedDomains) > 0
 
 	// Check whitelist first (takes precedence)
-	if len(s.config.Whitelist) > 0 {
+	if hasWhitelist {
 		for _, allowed := range s.config.Whitelist {
 			if strings.EqualFold(email, allowed) {
 				return true
@@ -255,14 +257,17 @@ func (s *Service) isEmailAllowed(email string) bool {
 	}
 
 	// Check allowed domains
-	if len(s.config.AllowedDomains) > 0 {
+	if hasAllowedDomains {
 		domain := stringutil.ExtractEmailDomain(email)
 		for _, allowed := range s.config.AllowedDomains {
 			if strings.EqualFold(domain, allowed) {
 				return true
 			}
 		}
-		// Domain not in allowed list
+	}
+
+	// If any restriction is configured but email didn't match, deny
+	if hasWhitelist || hasAllowedDomains {
 		return false
 	}
 
diff --git a/internal/service/oidcprovision/service_test.go b/internal/service/oidcprovision/service_test.go
@@ -480,6 +480,18 @@ func TestIsEmailAllowed(t *testing.T) {
 			email:          "user@partner.org",
 			expected:       true,
 		},
+		{
+			name:      "whitelist only - email not in whitelist - denied",
+			whitelist: []string{"allowed@example.com"},
+			email:     "other@example.com",
+			expected:  false,
+		},
+		{
+			name:      "whitelist only - email in whitelist - allowed",
+			whitelist: []string{"allowed@example.com"},
+			email:     "allowed@example.com",
+			expected:  true,
+		},
 	}
 
 	for _, tt := range tests {

-Original file line number
+Diff line change
 			email:          "[email protected]",
 			expected:       true,
 		},
 +		{
 +			name:      "whitelist only - email not in whitelist - denied",
 +			whitelist: []string{"[email protected]"},
 +			email:     "[email protected]",
 +			expected:  false,
 +		},
 +		{
 +			name:      "whitelist only - email in whitelist - allowed",
 +			whitelist: []string{"[email protected]"},
 +			email:     "[email protected]",
 +			expected:  true,
 +		},
+	}
 	for _, tt := range tests {