diff --git a/combined/cmd/config.go b/combined/cmd/config.go index 20a155319..97af10743 100644 --- a/combined/cmd/config.go +++ b/combined/cmd/config.go @@ -139,6 +139,7 @@ type AuthConfig struct { MfaSessionMaxLifetime string `yaml:"mfaSessionMaxLifetime"` MfaSessionIdleTimeout string `yaml:"mfaSessionIdleTimeout"` MfaSessionRememberMe bool `yaml:"mfaSessionRememberMe"` + SessionCookieEncryptionKey string `yaml:"sessionCookieEncryptionKey"` Storage AuthStorageConfig `yaml:"storage"` DashboardRedirectURIs []string `yaml:"dashboardRedirectURIs"` CLIRedirectURIs []string `yaml:"cliRedirectURIs"` @@ -585,13 +586,14 @@ func (c *CombinedConfig) buildEmbeddedIdPConfig(mgmt ManagementConfig) (*idp.Emb } cfg := &idp.EmbeddedIdPConfig{ - Enabled: true, - Issuer: mgmt.Auth.Issuer, - LocalAuthDisabled: mgmt.Auth.LocalAuthDisabled, - SignKeyRefreshEnabled: mgmt.Auth.SignKeyRefreshEnabled, - MfaSessionMaxLifetime: mgmt.Auth.MfaSessionMaxLifetime, - MfaSessionIdleTimeout: mgmt.Auth.MfaSessionIdleTimeout, - MfaSessionRememberMe: mgmt.Auth.MfaSessionRememberMe, + Enabled: true, + Issuer: mgmt.Auth.Issuer, + LocalAuthDisabled: mgmt.Auth.LocalAuthDisabled, + SignKeyRefreshEnabled: mgmt.Auth.SignKeyRefreshEnabled, + MfaSessionMaxLifetime: mgmt.Auth.MfaSessionMaxLifetime, + MfaSessionIdleTimeout: mgmt.Auth.MfaSessionIdleTimeout, + MfaSessionRememberMe: mgmt.Auth.MfaSessionRememberMe, + SessionCookieEncryptionKey: mgmt.Auth.SessionCookieEncryptionKey, Storage: idp.EmbeddedStorageConfig{ Type: authStorageType, Config: idp.EmbeddedStorageTypeConfig{ diff --git a/combined/config.yaml.example b/combined/config.yaml.example index b944895a1..66bc71703 100644 --- a/combined/config.yaml.example +++ b/combined/config.yaml.example @@ -90,6 +90,9 @@ server: # mfaSessionMaxLifetime: "24h" # Max duration for an MFA session from creation # mfaSessionIdleTimeout: "1h" # MFA session expires after this idle period # mfaSessionRememberMe: false # Pre-check "remember me" on login so the MFA session persists across tabs/restarts + # Optional AES key for encrypting embedded IdP session cookies. Can also be set via NB_IDP_SESSION_COOKIE_ENCRYPTION_KEY. + # Must be 16/24/32 raw bytes or base64-encoded to one of those lengths (for example: openssl rand -hex 16). + # sessionCookieEncryptionKey: "" # OAuth2 redirect URIs for dashboard dashboardRedirectURIs: - "https://app.example.com/nb-auth" diff --git a/idp/dex/provider.go b/idp/dex/provider.go index eb0c844ef..526d6a17a 100644 --- a/idp/dex/provider.go +++ b/idp/dex/provider.go @@ -511,13 +511,13 @@ func (p *Provider) SetClientsMFAChain(ctx context.Context, clientIDs []string, m // The handler expects requests with path prefix "/oauth2/". func (p *Provider) Handler() http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // NOTE: by default Dex will use the /logout route to only logout sessions, doesn't invalidate jwt tokens, - // to avoid confusion on users, we're not allowing for this, and only enable OIDC logout triggered through - // the dashboard which will invalidate both the session and the jwt token - //if strings.HasSuffix(r.URL.Path, "/logout") && r.FormValue("id_token_hint") == "" { - //http.Redirect(w, r, "/", http.StatusSeeOther) - //return - //} + // Dex's /logout endpoint requires id_token_hint for RP-initiated logout with + // post_logout_redirect_uri. If the dashboard calls logout without one, avoid + // rendering Dex's non-actionable Bad Request page and send the user home. + if strings.HasSuffix(r.URL.Path, "/logout") && r.FormValue("id_token_hint") == "" { + http.Redirect(w, r, "/", http.StatusSeeOther) + return + } p.dexServer.ServeHTTP(w, r) }) diff --git a/idp/dex/provider_test.go b/idp/dex/provider_test.go index 4ed89fd2e..88828fbbb 100644 --- a/idp/dex/provider_test.go +++ b/idp/dex/provider_test.go @@ -4,6 +4,8 @@ import ( "context" "encoding/json" "log/slog" + "net/http" + "net/http/httptest" "os" "path/filepath" "testing" @@ -144,6 +146,30 @@ func TestEncodeDexUserID_MatchesDexFormat(t *testing.T) { assert.Equal(t, knownEncodedID, reEncoded) } +func TestHandlerRedirectsLogoutWithoutIDTokenHint(t *testing.T) { + ctx := context.Background() + + tmpDir, err := os.MkdirTemp("", "dex-logout-handler-*") + require.NoError(t, err) + defer os.RemoveAll(tmpDir) + + provider, err := NewProvider(ctx, &Config{ + Issuer: "http://localhost:5556/oauth2", + Port: 5556, + DataDir: tmpDir, + }) + require.NoError(t, err) + defer func() { _ = provider.Stop(ctx) }() + + req := httptest.NewRequest(http.MethodGet, "/oauth2/logout?post_logout_redirect_uri=https://example.com", nil) + rec := httptest.NewRecorder() + + provider.Handler().ServeHTTP(rec, req) + + require.Equal(t, http.StatusSeeOther, rec.Code) + require.Equal(t, "/", rec.Header().Get("Location")) +} + func TestCreateUserInTempDB(t *testing.T) { ctx := context.Background() diff --git a/idp/dex/web/templates/password.html b/idp/dex/web/templates/password.html index 1d1b8282e..e1bfa7258 100755 --- a/idp/dex/web/templates/password.html +++ b/idp/dex/web/templates/password.html @@ -18,6 +18,7 @@ id="login" name="login" class="nb-input" + autocomplete="username" placeholder="Enter your {{ .UsernamePrompt | lower }}" {{ if .Username }}value="{{ .Username }}"{{ else }}autofocus{{ end }} required @@ -31,6 +32,7 @@ id="password" name="password" class="nb-input" + autocomplete="current-password" placeholder="Enter your password" {{ if .Invalid }}autofocus{{ end }} required diff --git a/management/server/idp/embedded.go b/management/server/idp/embedded.go index 54cb7d116..a1852a8bc 100644 --- a/management/server/idp/embedded.go +++ b/management/server/idp/embedded.go @@ -2,6 +2,7 @@ package idp import ( "context" + "encoding/base64" "errors" "fmt" "net/http" @@ -18,12 +19,13 @@ import ( ) const ( - staticClientDashboard = "netbird-dashboard" - staticClientCLI = "netbird-cli" - defaultCLIRedirectURL1 = "http://localhost:53000/" - defaultCLIRedirectURL2 = "http://localhost:54000/" - defaultScopes = "openid profile email groups" - defaultUserIDClaim = "sub" + staticClientDashboard = "netbird-dashboard" + staticClientCLI = "netbird-cli" + defaultCLIRedirectURL1 = "http://localhost:53000/" + defaultCLIRedirectURL2 = "http://localhost:54000/" + defaultScopes = "openid profile email groups" + defaultUserIDClaim = "sub" + sessionCookieEncryptionKeyEnv = "NB_IDP_SESSION_COOKIE_ENCRYPTION_KEY" ) // EmbeddedIdPConfig contains configuration for the embedded Dex OIDC identity provider @@ -60,6 +62,10 @@ type EmbeddedIdPConfig struct { // login screen. When true, the session cookie persists across browser tabs/restarts so // MFA is not re-prompted until the session expires. Defaults to false. MfaSessionRememberMe bool + // SessionCookieEncryptionKey is the optional AES key used to encrypt embedded IdP session cookies. + // It can also be set with NB_IDP_SESSION_COOKIE_ENCRYPTION_KEY. The value must be 16, 24, or 32 + // bytes when provided as a raw string, or base64-encoded to one of those lengths. + SessionCookieEncryptionKey string // Dashboard Post logout redirect URIs, these are required to tell // Dex what to allow when an RP-Initiated logout is started by the frontend // at least one of these must match the dashboard base URL or the dashboard @@ -188,7 +194,7 @@ func (c *EmbeddedIdPConfig) ToYAMLConfig() (*dex.YAMLConfig, error) { // Always initialize MFA providers and sessions so TOTP can be toggled at runtime. // MFAChain on clients is NOT set here — it's synced from the DB setting on startup. - if err := configureMFA(cfg, c.MfaSessionMaxLifetime, c.MfaSessionIdleTimeout, c.MfaSessionRememberMe); err != nil { + if err := configureMFA(cfg, c.MfaSessionMaxLifetime, c.MfaSessionIdleTimeout, c.MfaSessionRememberMe, c.SessionCookieEncryptionKey); err != nil { return nil, err } @@ -229,7 +235,7 @@ func sanitizePostLogoutRedirectURIs(uris []string) []string { return result } -func configureMFA(cfg *dex.YAMLConfig, sessionMaxLifetime, sessionIdleTimeout string, rememberMe bool) error { +func configureMFA(cfg *dex.YAMLConfig, sessionMaxLifetime, sessionIdleTimeout string, rememberMe bool, sessionCookieEncryptionKey string) error { cfg.MFA.Authenticators = []dex.MFAAuthenticator{{ ID: "default-totp", // Has to be caps otherwise it will fail @@ -247,13 +253,18 @@ func configureMFA(cfg *dex.YAMLConfig, sessionMaxLifetime, sessionIdleTimeout st sessionIdleTimeout = "1h" } + cookieEncryptionKey, err := resolveSessionCookieEncryptionKey(sessionCookieEncryptionKey) + if err != nil { + return err + } + cfg.Sessions = &dex.Sessions{ CookieName: "netbird-session", AbsoluteLifetime: sessionMaxLifetime, ValidIfNotUsedFor: sessionIdleTimeout, RememberMeCheckedByDefault: &rememberMe, SSOSharedWithDefault: "all", - CookieEncryptionKey: "32", + CookieEncryptionKey: cookieEncryptionKey, } // Absolutely required, otherwise the dex server will omit the MFA configuration entirely os.Setenv("DEX_SESSIONS_ENABLED", "true") @@ -263,6 +274,43 @@ func configureMFA(cfg *dex.YAMLConfig, sessionMaxLifetime, sessionIdleTimeout st return nil } +func resolveSessionCookieEncryptionKey(configuredKey string) (string, error) { + key := strings.TrimSpace(configuredKey) + if key == "" { + key = strings.TrimSpace(os.Getenv(sessionCookieEncryptionKeyEnv)) + } + if key == "" { + return "", nil + } + + if validSessionCookieEncryptionKeyLength(len([]byte(key))) { + return key, nil + } + + for _, encoding := range []*base64.Encoding{ + base64.StdEncoding, + base64.RawStdEncoding, + base64.URLEncoding, + base64.RawURLEncoding, + } { + decoded, err := encoding.DecodeString(key) + if err == nil && validSessionCookieEncryptionKeyLength(len(decoded)) { + return string(decoded), nil + } + } + + return "", fmt.Errorf("invalid embedded IdP session cookie encryption key: %s (or sessionCookieEncryptionKey) must be 16, 24, or 32 bytes as a raw string or base64-encoded to one of those lengths; got %d raw bytes", sessionCookieEncryptionKeyEnv, len([]byte(key))) +} + +func validSessionCookieEncryptionKeyLength(length int) bool { + switch length { + case 16, 24, 32: + return true + default: + return false + } +} + // Compile-time check that EmbeddedIdPManager implements Manager interface var _ Manager = (*EmbeddedIdPManager)(nil) diff --git a/management/server/idp/embedded_test.go b/management/server/idp/embedded_test.go index 4dda483fb..09dc67614 100644 --- a/management/server/idp/embedded_test.go +++ b/management/server/idp/embedded_test.go @@ -2,6 +2,7 @@ package idp import ( "context" + "encoding/base64" "os" "path/filepath" "testing" @@ -313,6 +314,72 @@ func TestEmbeddedIdPManager_UpdateUserPassword(t *testing.T) { }) } +func TestEmbeddedIdPConfig_ToYAMLConfig_SessionCookieEncryptionKey(t *testing.T) { + t.Setenv(sessionCookieEncryptionKeyEnv, "") + + rawKey := "0123456789abcdef0123456789abcdef" + config := &EmbeddedIdPConfig{ + Enabled: true, + Issuer: "http://localhost:5556/dex", + SessionCookieEncryptionKey: base64.StdEncoding.EncodeToString([]byte(rawKey)), + Storage: EmbeddedStorageConfig{ + Type: "sqlite3", + Config: EmbeddedStorageTypeConfig{ + File: filepath.Join(t.TempDir(), "dex.db"), + }, + }, + } + + yamlConfig, err := config.ToYAMLConfig() + require.NoError(t, err) + require.NotNil(t, yamlConfig.Sessions) + assert.Equal(t, rawKey, yamlConfig.Sessions.CookieEncryptionKey) +} + +func TestResolveSessionCookieEncryptionKey(t *testing.T) { + rawKey := "0123456789abcdef0123456789abcdef" + + t.Run("uses raw configured key", func(t *testing.T) { + t.Setenv(sessionCookieEncryptionKeyEnv, "") + + key, err := resolveSessionCookieEncryptionKey(rawKey) + require.NoError(t, err) + assert.Equal(t, rawKey, key) + }) + + t.Run("uses base64 configured key", func(t *testing.T) { + t.Setenv(sessionCookieEncryptionKeyEnv, "") + + key, err := resolveSessionCookieEncryptionKey(base64.StdEncoding.EncodeToString([]byte(rawKey))) + require.NoError(t, err) + assert.Equal(t, rawKey, key) + }) + + t.Run("falls back to env var", func(t *testing.T) { + t.Setenv(sessionCookieEncryptionKeyEnv, rawKey) + + key, err := resolveSessionCookieEncryptionKey("") + require.NoError(t, err) + assert.Equal(t, rawKey, key) + }) + + t.Run("empty key disables encryption", func(t *testing.T) { + t.Setenv(sessionCookieEncryptionKeyEnv, "") + + key, err := resolveSessionCookieEncryptionKey("") + require.NoError(t, err) + assert.Empty(t, key) + }) + + t.Run("rejects invalid key length", func(t *testing.T) { + t.Setenv(sessionCookieEncryptionKeyEnv, "") + + _, err := resolveSessionCookieEncryptionKey("32") + require.Error(t, err) + assert.Contains(t, err.Error(), sessionCookieEncryptionKeyEnv) + }) +} + func TestEmbeddedIdPManager_GetLocalKeysLocation(t *testing.T) { ctx := context.Background()