Skip to content

Commit 7ad15a4

Browse files
ericfitzclaude
andauthored
fix: Heroku deployment issues - OAuth provider discovery and UserPreference migration (#122)
* fix(config): use dynamic OAuth provider discovery from environment variables The overrideOAuthProviders function was only looking for providers that already existed in the config map with hardcoded names (google, github, microsoft). When no YAML config file was loaded (as on Heroku), the providers map was empty and no providers would be discovered. This change makes overrideOAuthProviders work like overrideSAMLProviders by using envutil.DiscoverProviders to dynamically find all OAuth providers from environment variables matching the pattern OAUTH_PROVIDERS_<ID>_ENABLED. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(api): add UserPreference model to GetAllModels() for migrations The GetAllModels() function in api/store.go was missing the UserPreference model, which caused the user_preferences table to not be created during GORM AutoMigrate on server startup. This aligns GetAllModels() with AllModels() in api/models/models.go which has all 25 models including UserPreference. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(test): add return after t.Fatal to satisfy staticcheck SA5011 Staticcheck SA5011 warns about possible nil pointer dereference even after t.Fatal() because it doesn't understand that t.Fatal terminates execution. Adding explicit return statements after t.Fatal() makes the control flow clear to the linter. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 93f28e4 commit 7ad15a4

File tree

5 files changed

+64
-35
lines changed

5 files changed

+64
-35
lines changed

.version

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
22
"major": 0,
33
"minor": 276,
4-
"patch": 5
4+
"patch": 8
55
}

api/health_checker_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ func TestHealthChecker_NewHealthChecker(t *testing.T) {
3232

3333
if checker == nil {
3434
t.Fatal("expected non-nil health checker")
35+
return // staticcheck: make it clear execution stops here
3536
}
3637
if checker.timeout != timeout {
3738
t.Errorf("expected timeout %v, got %v", timeout, checker.timeout)
@@ -49,6 +50,7 @@ func TestComponentHealthResult_ToAPIComponentHealth(t *testing.T) {
4950

5051
if apiHealth == nil {
5152
t.Fatal("expected non-nil API component health")
53+
return // staticcheck: make it clear execution stops here
5254
}
5355
if apiHealth.Status != ComponentHealthStatusHealthy {
5456
t.Errorf("expected healthy status, got %s", apiHealth.Status)

api/store.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,5 +158,6 @@ func GetAllModels() []interface{} {
158158
&models.AddonInvocationQuota{},
159159
&models.UserAPIQuota{},
160160
&models.GroupMember{},
161+
&models.UserPreference{},
161162
}
162163
}

api/version.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ var (
2929
// Minor version number
3030
VersionMinor = "276"
3131
// Patch version number
32-
VersionPatch = "5"
32+
VersionPatch = "8"
3333
// GitCommit is the git commit hash from build
3434
GitCommit = "development"
3535
// BuildDate is the build timestamp

internal/config/config.go

Lines changed: 59 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -445,53 +445,79 @@ func overrideStructWithEnv(v reflect.Value) error {
445445

446446
// overrideOAuthProviders handles environment variable overrides for OAuth providers
447447
func overrideOAuthProviders(mapField reflect.Value) error {
448+
logger := slogging.Get()
449+
logger.Info("[CONFIG] overrideOAuthProviders called - starting dynamic OAuth provider discovery")
450+
448451
if mapField.IsNil() {
449-
return nil
452+
logger.Info("[CONFIG] OAuth providers map is nil, initializing it")
453+
mapField.Set(reflect.MakeMap(mapField.Type()))
450454
}
451455

452-
providers := []string{"google", "github", "microsoft"}
456+
// Discover OAuth providers from environment variables
457+
providerIDs := envutil.DiscoverProviders("OAUTH_PROVIDERS_", "_ENABLED")
458+
logger.Info("[CONFIG] Discovered %d OAuth provider IDs: %v", len(providerIDs), providerIDs)
453459

454-
for _, providerID := range providers {
455-
providerValue := mapField.MapIndex(reflect.ValueOf(providerID))
456-
if !providerValue.IsValid() {
460+
for _, providerID := range providerIDs {
461+
envPrefix := fmt.Sprintf("OAUTH_PROVIDERS_%s_", providerID)
462+
463+
// Check if this provider is enabled
464+
enabledStr := os.Getenv(envPrefix + "ENABLED")
465+
if enabledStr != "true" {
466+
logger.Info("[CONFIG] OAuth provider %s is not enabled (ENABLED=%s), skipping", providerID, enabledStr)
457467
continue
458468
}
459469

460-
// Create a copy of the provider config to modify
461-
provider := providerValue.Interface().(OAuthProviderConfig)
462-
463-
// Override provider-specific environment variables
464-
envPrefix := fmt.Sprintf("OAUTH_PROVIDERS_%s_", strings.ToUpper(providerID))
465-
466-
if val := os.Getenv(envPrefix + "ENABLED"); val != "" {
467-
provider.Enabled = val == "true"
468-
}
469-
if val := os.Getenv(envPrefix + "ICON"); val != "" {
470-
provider.Icon = val
471-
}
472-
if val := os.Getenv(envPrefix + "CLIENT_ID"); val != "" {
473-
provider.ClientID = val
474-
}
475-
if val := os.Getenv(envPrefix + "CLIENT_SECRET"); val != "" {
476-
provider.ClientSecret = val
477-
}
478-
if val := os.Getenv(envPrefix + "AUTHORIZATION_URL"); val != "" {
479-
provider.AuthorizationURL = val
470+
// Convert provider ID to key (e.g., "GOOGLE" -> "google", "GITHUB" -> "github")
471+
providerKey := envutil.ProviderIDToKey(providerID)
472+
logger.Info("[CONFIG] Processing OAuth provider: %s (key: %s)", providerID, providerKey)
473+
474+
// Parse scopes (comma-separated)
475+
scopesStr := os.Getenv(envPrefix + "SCOPES")
476+
var scopes []string
477+
if scopesStr != "" {
478+
scopes = strings.Split(scopesStr, ",")
479+
for i := range scopes {
480+
scopes[i] = strings.TrimSpace(scopes[i])
481+
}
480482
}
481-
if val := os.Getenv(envPrefix + "TOKEN_URL"); val != "" {
482-
provider.TokenURL = val
483+
484+
// Build userinfo endpoints array
485+
var userInfoEndpoints []UserInfoEndpoint
486+
if userinfoURL := os.Getenv(envPrefix + "USERINFO_URL"); userinfoURL != "" {
487+
userInfoEndpoints = append(userInfoEndpoints, UserInfoEndpoint{
488+
URL: userinfoURL,
489+
})
483490
}
484-
if val := os.Getenv(envPrefix + "ISSUER"); val != "" {
485-
provider.Issuer = val
491+
492+
// Create new OAuth provider config
493+
provider := OAuthProviderConfig{
494+
ID: providerKey,
495+
Name: os.Getenv(envPrefix + "NAME"),
496+
Enabled: true,
497+
Icon: os.Getenv(envPrefix + "ICON"),
498+
ClientID: os.Getenv(envPrefix + "CLIENT_ID"),
499+
ClientSecret: os.Getenv(envPrefix + "CLIENT_SECRET"),
500+
AuthorizationURL: os.Getenv(envPrefix + "AUTHORIZATION_URL"),
501+
TokenURL: os.Getenv(envPrefix + "TOKEN_URL"),
502+
Issuer: os.Getenv(envPrefix + "ISSUER"),
503+
JWKSURL: os.Getenv(envPrefix + "JWKS_URL"),
504+
Scopes: scopes,
505+
UserInfo: userInfoEndpoints,
486506
}
487-
if val := os.Getenv(envPrefix + "JWKS_URL"); val != "" {
488-
provider.JWKSURL = val
507+
508+
// Use key as default name if not set
509+
if provider.Name == "" {
510+
provider.Name = providerKey
489511
}
490512

491-
// Set the modified provider back to the map
492-
mapField.SetMapIndex(reflect.ValueOf(providerID), reflect.ValueOf(provider))
513+
logger.Info("[CONFIG] Adding OAuth provider %s to map (ID: %s, Name: %s, ClientID set: %v)",
514+
providerKey, provider.ID, provider.Name, provider.ClientID != "")
515+
516+
// Set the provider in the map
517+
mapField.SetMapIndex(reflect.ValueOf(providerKey), reflect.ValueOf(provider))
493518
}
494519

520+
logger.Info("[CONFIG] OAuth provider discovery complete, %d providers in map", mapField.Len())
495521
return nil
496522
}
497523

0 commit comments

Comments
 (0)