From c7a26d81fe0a4e159396e59611a7aa0eab032a3a Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Sun, 31 Aug 2025 07:17:50 +0000 Subject: [PATCH] fix(auth): do not panic with invalid openid provider configuration (#1354) --- pkg/modules/auth/openid/providers.go | 28 ++++++-- pkg/modules/auth/openid/providers_test.go | 86 +++++++++++++++++++++++ 2 files changed, 108 insertions(+), 6 deletions(-) create mode 100644 pkg/modules/auth/openid/providers_test.go diff --git a/pkg/modules/auth/openid/providers.go b/pkg/modules/auth/openid/providers.go index 113a97d16..5638d0316 100644 --- a/pkg/modules/auth/openid/providers.go +++ b/pkg/modules/auth/openid/providers.go @@ -44,8 +44,18 @@ func GetAllProviders() (providers []*Provider, err error) { rawProvider, is := rawProviders.(map[string]interface{}) if !is { - log.Criticalf("It looks like your openid configuration is in the wrong format. Please check the docs for the correct format.") - return + // Try to convert from map[interface{}]interface{} (YAML format) + if rawProviderInterface, ok := rawProviders.(map[interface{}]interface{}); ok { + rawProvider = make(map[string]interface{}, len(rawProviderInterface)) + for k, v := range rawProviderInterface { + if key, keyOK := k.(string); keyOK { + rawProvider[key] = v + } + } + } else { + log.Criticalf("It looks like your openid configuration is in the wrong format. Please check the docs for the correct format.") + return + } } for key, p := range rawProvider { @@ -55,10 +65,16 @@ func GetAllProviders() (providers []*Provider, err error) { // JSON config is a map[string]interface{}, other providers are not. Under the hood they are all strings so // it is safe to cast. if !is { - pis := p.(map[interface{}]interface{}) - pi = make(map[string]interface{}, len(pis)) - for i, s := range pis { - pi[i.(string)] = s + if pis, pisOK := p.(map[interface{}]interface{}); pisOK { + pi = make(map[string]interface{}, len(pis)) + for i, s := range pis { + if key, keyOK := i.(string); keyOK { + pi[key] = s + } + } + } else { + log.Errorf("Provider %s has invalid configuration format, skipping", key) + continue } } diff --git a/pkg/modules/auth/openid/providers_test.go b/pkg/modules/auth/openid/providers_test.go new file mode 100644 index 000000000..377ae548c --- /dev/null +++ b/pkg/modules/auth/openid/providers_test.go @@ -0,0 +1,86 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-present Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package openid + +import ( + "testing" + + "code.vikunja.io/api/pkg/config" + "code.vikunja.io/api/pkg/modules/keyvalue" +) + +func TestGetAllProvidersTypeSafety(t *testing.T) { + // Clean up any existing providers + defer func() { + CleanupSavedOpenIDProviders() + }() + + t.Run("should handle []interface{} without panic", func(t *testing.T) { + // Setup config with OpenID enabled + config.AuthOpenIDEnabled.Set(true) + + // Mock the config value to be []interface{} which causes the original panic + configValue := []interface{}{ + map[string]interface{}{ + "name": "test-provider", + "authurl": "https://example.com/auth", + "clientid": "test-client", + "clientsecret": "test-secret", + }, + } + config.AuthOpenIDProviders.Set(configValue) + + // Clear keyvalue cache to force reading from config + _ = keyvalue.Del("openid_providers") + + // This should not panic, but should handle gracefully and return empty + providers, err := GetAllProviders() + + // Should return empty providers since the config format is invalid + if err != nil { + t.Errorf("Expected no error, got: %v", err) + } + + if len(providers) != 0 { + t.Errorf("Expected empty providers list, got: %d", len(providers)) + } + }) + + t.Run("should handle other invalid types without panic", func(t *testing.T) { + // Setup config with OpenID enabled + config.AuthOpenIDEnabled.Set(true) + + // Mock the config value to be a string (another invalid type) + configValue := "invalid-config" + config.AuthOpenIDProviders.Set(configValue) + + // Clear keyvalue cache to force reading from config + _ = keyvalue.Del("openid_providers") + + // This should not panic + providers, err := GetAllProviders() + + // Should return empty providers since the config format is invalid + if err != nil { + t.Errorf("Expected no error, got: %v", err) + } + + if len(providers) != 0 { + t.Errorf("Expected empty providers list, got: %d", len(providers)) + } + }) +}