diff --git a/.env.example b/.env.example index 45d488e1..be207216 100644 --- a/.env.example +++ b/.env.example @@ -182,6 +182,8 @@ TINYAUTH_OAUTH_PROVIDERS_name_CLAIMS_GROUPS= # oidc config +# Enable legacy username-based OIDC subject identifiers for backwards compatibility. +TINYAUTH_OIDC_LEGACYSUBENABLED=true # Path to the private key file, including file name. TINYAUTH_OIDC_PRIVATEKEYPATH="./tinyauth_oidc_key" # Path to the public key file, including file name. diff --git a/cmd/tinyauth/tinyauth.go b/cmd/tinyauth/tinyauth.go index 506cb0ca..cd45affb 100644 --- a/cmd/tinyauth/tinyauth.go +++ b/cmd/tinyauth/tinyauth.go @@ -3,7 +3,6 @@ package main import ( "fmt" "os" - "reflect" "strings" "charm.land/huh/v2" @@ -32,10 +31,13 @@ func main() { Configuration: tConfig, Resources: loaders, Run: func(_ []string) error { - // enable this on experimental features - if !reflect.DeepEqual(model.NewDefaultConfiguration(env).Experimental, tConfig.Experimental) { - colors := getColors() - fmt.Println(colors.yellow.Render("⚠") + " Experimental features are enabled, use with caution. Experimental features may change with each release.") + colors := getColors() + res := tConfig.Validate() + if len(res.Errors) > 0 { + return fmt.Errorf("invalid configuration: %s", strings.Join(res.Errors, ", ")) + } + for _, warn := range res.Warnings { + fmt.Println(colors.yellow.Render("⚠") + " " + warn) } return runCmd(*tConfig) }, diff --git a/internal/bootstrap/app_bootstrap.go b/internal/bootstrap/app_bootstrap.go index 6fc95408..a8e788e8 100644 --- a/internal/bootstrap/app_bootstrap.go +++ b/internal/bootstrap/app_bootstrap.go @@ -154,11 +154,6 @@ func (app *BootstrapApp) Setup() error { app.runtime.OAuthProviders[id] = provider } - // cookie domain - if !app.config.Auth.SubdomainsEnabled { - app.log.App.Warn().Msg("Subdomains are disabled, cookies will be set for the current domain only") - } - cookieDomain, err := utils.GetCookieDomain(app.runtime.AppURL, app.config.Auth.SubdomainsEnabled) if err != nil { diff --git a/internal/model/config.go b/internal/model/config.go index 9d514390..e535adc7 100644 --- a/internal/model/config.go +++ b/internal/model/config.go @@ -2,6 +2,7 @@ package model import ( "os" + "reflect" "time" ) @@ -80,8 +81,9 @@ func NewDefaultConfiguration(runtimeEnv RuntimeEnv) *Config { }, }, OIDC: OIDCConfig{ - PrivateKeyPath: "./tinyauth_oidc_key", - PublicKeyPath: "./tinyauth_oidc_key.pub", + LegacySubEnabled: true, + PrivateKeyPath: "./tinyauth_oidc_key", + PublicKeyPath: "./tinyauth_oidc_key.pub", }, Tailscale: TailscaleConfig{ CacheDuration: int(time.Duration(5 * time.Minute).Seconds()), @@ -196,9 +198,10 @@ type OAuthConfig struct { } type OIDCConfig struct { - PrivateKeyPath string `description:"Path to the private key file, including file name." yaml:"privateKeyPath,omitempty"` - PublicKeyPath string `description:"Path to the public key file, including file name." yaml:"publicKeyPath,omitempty"` - Clients map[string]OIDCClientConfig `description:"OIDC clients configuration." yaml:"clients,omitempty"` + LegacySubEnabled bool `description:"Enable legacy username-based OIDC subject identifiers for backwards compatibility." yaml:"legacySubEnabled,omitempty"` + PrivateKeyPath string `description:"Path to the private key file, including file name." yaml:"privateKeyPath,omitempty"` + PublicKeyPath string `description:"Path to the public key file, including file name." yaml:"publicKeyPath,omitempty"` + Clients map[string]OIDCClientConfig `description:"OIDC clients configuration." yaml:"clients,omitempty"` } type UIConfig struct { @@ -345,3 +348,35 @@ type AppPath struct { Allow string `description:"Disable authentication for only paths that match the regex string." yaml:"allow,omitempty"` Block string `description:"Enable authentication for only paths that match the regex string." yaml:"block,omitempty"` } + +type ValidateResult struct { + Warnings []string `json:"warnings"` + Errors []string `json:"errors"` +} + +// Helper config functions + +func (c *Config) Validate() ValidateResult { + res := ValidateResult{ + Warnings: make([]string, 0), + Errors: make([]string, 0), + } + env := DetectRuntimeEnv() + + // warn on experimental features + if !reflect.DeepEqual(NewDefaultConfiguration(env).Experimental, c.Experimental) { + res.Warnings = append(res.Warnings, "Experimental features are enabled, use with caution. Experimental features may change with each release") + } + + // warn on subdomains disabled + if !c.Auth.SubdomainsEnabled { + res.Warnings = append(res.Warnings, "Subdomains are disabled, cookies will be set for the current domain only") + } + + // warn on legacy sub + if c.OIDC.LegacySubEnabled { + res.Warnings = append(res.Warnings, "Legacy username-based OIDC subject identifiers are enabled, this is insecure and will be removed in the next major release") + } + + return res +} diff --git a/internal/service/oidc_service.go b/internal/service/oidc_service.go index c5f8ecd3..b5098a68 100644 --- a/internal/service/oidc_service.go +++ b/internal/service/oidc_service.go @@ -896,7 +896,15 @@ func (service *OIDCService) hashAndEncodePKCE(codeVerifier string) string { // We will just create a uuid out of the username and client name which remains stable, // but if username or client name changes then sub changes too. func (service *OIDCService) CreateSub(userContext model.UserContext, clientId string) string { - return utils.GenerateUUID(fmt.Sprintf("%s:%s", userContext.GetUsername(), clientId)) + sub := fmt.Sprintf("%q:%q:%q", userContext.GetProviderID(), userContext.GetUsername(), clientId) + + // The old sub created by the username and client ID is insecure + // because it allows subs from different providers to be the same + if service.config.OIDC.LegacySubEnabled { + sub = fmt.Sprintf("%s:%s", userContext.GetUsername(), clientId) + } + + return utils.GenerateUUID(sub) } func (service *OIDCService) IsCodeUsed(codeHash string) (string, bool) {