Merge remote-tracking branch 'origin/master'

# Conflicts: # coverage/coverage.out
2025-12-06 06:22:41 -08:00 · 2025-11-03 20:41:41 +01:00
parent 84f4e3a1ba 27c26db783
commit a780509db2
8 changed files with 176 additions and 19 deletions
--- a/README.md
+++ b/README.md
@@ -202,6 +202,7 @@ You can specify configuration options either via a config file (default: `config
 | `security.password_reset_max_rate` /<br> `WAKAPI_PASSWORD_RESET_MAX_RATE`                   | `5/1h`                                           | Rate limiting config for password reset endpoint in format `<max_req>/<multiplier><unit>`, where `unit` is one of `s`, `m` or `h`.                                              |
 | `security.oidc`                                                                             | `[]`                                             | List of OpenID Connect provider configurations (for details, see [wiki](https://github.com/muety/wakapi/wiki/OpenID-Connect-login-(SSO)))                                       |
 | `security.oidc[0].name` /<br> `WAKAPI_OIDC_PROVIDERS_0_NAME`                                | -                                                | Name / identifier for the OpenID Connect provider (e.g. `gitlab`)                                                                                                               |
+| `security.oidc[0].display_name` /<br> `WAKAPI_OIDC_PROVIDERS_0_DISPLAY_NAME`                | -                                                | Optional "human-readable" display name for the provider presented to the user                                                                                                   |
 | `security.oidc[0].client_id` /<br> `WAKAPI_OIDC_PROVIDERS_0_CLIENT_ID`                      | -                                                | OAuth client name with this provider                                                                                                                                            |
 | `security.oidc[0].client_secret` /<br> `WAKAPI_OIDC_PROVIDERS_0_CLIENT_SECRET`              | -                                                | OAuth client secret with this provider                                                                                                                                          |
 | `security.oidc[0].endpoint` /<br> `WAKAPI_OIDC_PROVIDERS_0_ENDPOINT`                        | -                                                | OpenID Connect provider API entrypoint (for [discovery](https://openid.net/specs/openid-connect-discovery-1_0.html))                                                            |
--- a/config/config.go
+++ b/config/config.go
@@ -12,6 +12,7 @@ import (
 	"time"

 	"github.com/duke-git/lancet/v2/slice"
+	"github.com/duke-git/lancet/v2/strutil"

 	"log/slog"

@@ -207,6 +208,7 @@ type SMTPMailConfig struct {
 type oidcProviderConfig struct {
 	// for environment variables format, see renameEnvVars() down below
 	Name         string `yaml:"name"`
+	DisplayName  string `yaml:"display_name"` // optional
 	ClientID     string `yaml:"client_id"`
 	ClientSecret string `yaml:"client_secret"`
 	Endpoint     string `yaml:"endpoint"` // base url from which auto-discovery (.well-known/openid-configuration) can be found
@@ -228,6 +230,32 @@ type Config struct {
 	Mail           mailConfig
 }

+func (c *oidcProviderConfig) String() string {
+	if c.DisplayName != "" {
+		return c.DisplayName
+	}
+	return strutil.Capitalize(c.Name)
+}
+
+func (c *oidcProviderConfig) Validate() error {
+	var namePattern = regexp.MustCompile("^[a-zA-Z0-9-]+$")
+	var endpointPattern = regexp.MustCompile("^https?://")
+
+	if !namePattern.MatchString(c.Name) {
+		return fmt.Errorf("invalid provider name '%s', must only contain alphanumeric characters or '-'", c.Name)
+	}
+	if c.ClientID == "" {
+		return fmt.Errorf("provider '%s' is missing client id", c.Name)
+	}
+	if c.ClientSecret == "" {
+		return fmt.Errorf("provider '%s' is missing client secret", c.Name)
+	}
+	if !endpointPattern.MatchString(c.Endpoint) {
+		return fmt.Errorf("provider '%s' is missing endpoint", c.Name)
+	}
+	return nil
+}
+
 func (c *Config) AppStartTimestamp() string {
 	return fmt.Sprintf("%d", appStartTime.Unix())
 }
@@ -627,6 +655,11 @@ func Load(configFlag string, version string) *Config {
 	if d, err := time.Parse(config.App.DateTimeFormat, config.App.DateTimeFormat); err != nil || !d.Equal(time.Date(2006, time.January, 2, 15, 4, 0, 0, d.Location())) {
 		Log().Fatal("invalid datetime format", "format", config.App.DateTimeFormat)
 	}
+	for _, provider := range config.Security.OidcProviders {
+		if err := provider.Validate(); err != nil {
+			Log().Fatal("invalid oidc provider config", "provider", provider.Name, "error", err)
+		}
+	}

 	cronParser := cron.NewParser(cron.Second | cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow | cron.Descriptor)

--- a/config/config_test.go
+++ b/config/config_test.go
@@ -19,6 +19,7 @@ func Test_Load_OidcProviders(t *testing.T) {
 	defer oidcMock2.Shutdown()

 	os.Setenv("WAKAPI_OIDC_PROVIDERS_0_NAME", "testprovider1")
+	os.Setenv("WAKAPI_OIDC_PROVIDERS_0_DISPLAY_NAME", "Test Provider 1")
 	os.Setenv("WAKAPI_OIDC_PROVIDERS_0_CLIENT_ID", oidcMock1.ClientID)
 	os.Setenv("WAKAPI_OIDC_PROVIDERS_0_CLIENT_SECRET", oidcMock1.ClientSecret)
 	os.Setenv("WAKAPI_OIDC_PROVIDERS_0_ENDPOINT", oidcMock1.Addr()+"/oidc")
@@ -32,19 +33,127 @@ func Test_Load_OidcProviders(t *testing.T) {

 	assert.Len(t, oidcCfg, 2)
 	assert.Equal(t, "testprovider1", oidcCfg[0].Name)
+	assert.Equal(t, "Test Provider 1", oidcCfg[0].DisplayName)
+	assert.Equal(t, "Test Provider 1", oidcCfg[0].String())
 	assert.Equal(t, oidcMock1.ClientID, oidcCfg[0].ClientID)
 	assert.Equal(t, oidcMock1.ClientSecret, oidcCfg[0].ClientSecret)
 	assert.Equal(t, oidcMock1.Addr()+"/oidc", oidcCfg[0].Endpoint)
 	assert.Equal(t, "testprovider2", oidcCfg[1].Name)
+	assert.Equal(t, "", oidcCfg[1].DisplayName)
+	assert.Equal(t, "Testprovider2", oidcCfg[1].String())
 	assert.Equal(t, oidcMock2.ClientID, oidcCfg[1].ClientID)
 	assert.Equal(t, oidcMock2.ClientSecret, oidcCfg[1].ClientSecret)
 	assert.Equal(t, oidcMock2.Addr()+"/oidc", oidcCfg[1].Endpoint)

-	_, err1 := GetOidcProvider("testprovider1")
-	_, err2 := GetOidcProvider("testprovider2")
-
+	p1, err1 := GetOidcProvider("testprovider1")
 	assert.Nil(t, err1)
+	assert.Equal(t, "Test Provider 1", p1.DisplayName)
+
+	p2, err2 := GetOidcProvider("testprovider2")
 	assert.Nil(t, err2)
+	assert.Equal(t, "Testprovider2", p2.DisplayName)
+}
+
+func TestOidcProviderConfig_Validate(t *testing.T) {
+	// note: test cases were generated by ai
+	testCases := []struct {
+		name   string
+		config oidcProviderConfig
+		err    string
+	}{
+		{
+			name: "valid",
+			config: oidcProviderConfig{
+				Name:         "test-provider-1",
+				ClientID:     "client-id",
+				ClientSecret: "client-secret",
+				Endpoint:     "https://provider.com/oidc",
+			},
+			err: "",
+		},
+		{
+			name: "valid with http",
+			config: oidcProviderConfig{
+				Name:         "test-provider-1",
+				ClientID:     "client-id",
+				ClientSecret: "client-secret",
+				Endpoint:     "http://provider.com/oidc",
+			},
+			err: "",
+		},
+		{
+			name: "invalid name with spaces",
+			config: oidcProviderConfig{
+				Name: "test provider",
+			},
+			err: "invalid provider name 'test provider', must only contain alphanumeric characters or '-'",
+		},
+		{
+			name: "invalid name with underscore",
+			config: oidcProviderConfig{
+				Name: "test_provider",
+			},
+			err: "invalid provider name 'test_provider', must only contain alphanumeric characters or '-'",
+		},
+		{
+			name: "missing client id",
+			config: oidcProviderConfig{
+				Name:         "test-provider",
+				ClientSecret: "client-secret",
+				Endpoint:     "https://provider.com/oidc",
+			},
+			err: "provider 'test-provider' is missing client id",
+		},
+		{
+			name: "missing client secret",
+			config: oidcProviderConfig{
+				Name:     "test-provider",
+				ClientID: "client-id",
+				Endpoint: "https://provider.com/oidc",
+			},
+			err: "provider 'test-provider' is missing client secret",
+		},
+		{
+			name: "missing endpoint",
+			config: oidcProviderConfig{
+				Name:         "test-provider",
+				ClientID:     "client-id",
+				ClientSecret: "client-secret",
+			},
+			err: "provider 'test-provider' is missing endpoint",
+		},
+		{
+			name: "invalid endpoint scheme",
+			config: oidcProviderConfig{
+				Name:         "test-provider",
+				ClientID:     "client-id",
+				ClientSecret: "client-secret",
+				Endpoint:     "ftp://provider.com/oidc",
+			},
+			err: "provider 'test-provider' is missing endpoint",
+		},
+		{
+			name: "endpoint without scheme",
+			config: oidcProviderConfig{
+				Name:         "test-provider",
+				ClientID:     "client-id",
+				ClientSecret: "client-secret",
+				Endpoint:     "provider.com/oidc",
+			},
+			err: "provider 'test-provider' is missing endpoint",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := tc.config.Validate()
+			if tc.err == "" {
+				assert.NoError(t, err)
+			} else {
+				assert.EqualError(t, err, tc.err)
+			}
+		})
+	}
 }

 func TestConfig_IsDev(t *testing.T) {
--- a/config/oidc.go
+++ b/config/oidc.go
@@ -11,9 +11,10 @@ import (
 )

 type OidcProvider struct {
-	Name     string
-	OAuth2   *oauth2.Config
-	Verifier *oidc.IDTokenVerifier
+	Name        string
+	DisplayName string
+	OAuth2      *oauth2.Config
+	Verifier    *oidc.IDTokenVerifier
 }

 type IdTokenPayload struct {
@@ -70,9 +71,10 @@ func RegisterOidcProvider(providerCfg *oidcProviderConfig) {
 	}

 	oidcProviders[providerCfg.Name] = &OidcProvider{
-		Name:     providerCfg.Name,
-		OAuth2:   &oauth2Conf,
-		Verifier: provider.Verifier(&oidc.Config{ClientID: providerCfg.ClientID}),
+		Name:        providerCfg.Name,
+		DisplayName: providerCfg.String(),
+		OAuth2:      &oauth2Conf,
+		Verifier:    provider.Verifier(&oidc.Config{ClientID: providerCfg.ClientID}),
 	}
 }

--- a/models/view/login.go
+++ b/models/view/login.go
@@ -6,7 +6,12 @@ type LoginViewModel struct {
 	AllowSignup   bool
 	CaptchaId     string
 	InviteCode    string
-	OidcProviders []string
+	OidcProviders []LoginViewModelOidcProvider
+}
+
+type LoginViewModelOidcProvider struct {
+	Name        string
+	DisplayName string
 }

 type SetPasswordViewModel struct {
--- a/routes/login.go
+++ b/routes/login.go
@@ -10,6 +10,7 @@ import (

 	"github.com/dchest/captcha"
 	"github.com/duke-git/lancet/v2/random"
+	"github.com/duke-git/lancet/v2/slice"
 	"github.com/go-chi/chi/v5"
 	"github.com/go-chi/httprate"
 	conf "github.com/muety/wakapi/config"
@@ -475,7 +476,13 @@ func (h *LoginHandler) buildViewModel(r *http.Request, w http.ResponseWriter, wi
 		TotalUsers:      int(numUsers),
 		AllowSignup:     h.config.IsDev() || h.config.Security.AllowSignup,
 		InviteCode:      r.URL.Query().Get("invite"),
-		OidcProviders:   h.config.Security.ListOidcProviders(),
+		OidcProviders: slice.Map(h.config.Security.ListOidcProviders(), func(i int, providerName string) view.LoginViewModelOidcProvider {
+			provider, _ := conf.GetOidcProvider(providerName) // no error, because only using registered provider names
+			return view.LoginViewModelOidcProvider{
+				Name:        provider.Name,
+				DisplayName: provider.DisplayName,
+			}
+		}),
 	}

 	if withCaptcha {
--- a/views/login.tpl.html
+++ b/views/login.tpl.html
@@ -49,12 +49,12 @@
        <div class="mt-10">
            <div class="font-semibold mb-2">Single Sign-On</div>
            {{ range $provider := .OidcProviders }}
-            <a href="oidc/{{ $provider | lower }}/login" class="block mb-2">
+            <a href="oidc/{{ $provider.Name | lower }}/login" class="block mb-2">
                <button type="button" class="btn-default w-full flex items-center gap-2 justify-center">
-                    {{ if $.OidcProviderIcon $provider }}
-                    <span class="iconify inline text-white text-base" data-icon="{{ ($.OidcProviderIcon $provider) | urlSafe }}"></span>
+                    {{ if $.OidcProviderIcon $provider.Name }}
+                    <span class="iconify inline text-white text-base" data-icon="{{ ($.OidcProviderIcon $provider.Name) | urlSafe }}"></span>
                    {{ end }}
-                    Login with {{ $provider | capitalize }}
+                    Login with {{ $provider.DisplayName }}
                </button>
            </a>
            {{ end }}
--- a/views/signup.tpl.html
+++ b/views/signup.tpl.html
@@ -114,12 +114,12 @@
        <div class="mt-10">
            <div class="font-semibold mb-2">Single Sign-On</div>
            {{ range $provider := .OidcProviders }}
-            <a href="oidc/{{ $provider | lower }}/login" class="block mb-2">
+            <a href="oidc/{{ $provider.Name | lower }}/login" class="block mb-2">
                <button type="button" class="btn-default w-full flex items-center gap-2 justify-center">
-                    {{ if $.OidcProviderIcon $provider }}
-                    <span class="iconify inline text-white text-base" data-icon="{{ ($.OidcProviderIcon $provider) | urlSafe }}"></span>
+                    {{ if $.OidcProviderIcon $provider.Name }}
+                    <span class="iconify inline text-white text-base" data-icon="{{ ($.OidcProviderIcon $provider.Name) | urlSafe }}"></span>
                    {{ end }}
-                    Sign up {{ $provider | capitalize }}
+                    Sign up {{ $provider.DisplayName }}
                </button>
            </a>
            {{ end }}