Merge branch 'main' into fix/quickstart-subnet-and-domain-alias

client/cmd/login.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"os"
 	"os/user"
+	"runtime"
 	"strings"
 
 	log "github.com/sirupsen/logrus"
@@ -120,7 +121,7 @@ func doDaemonLogin(ctx context.Context, cmd *cobra.Command, providedSetupKey str
 	loginRequest := proto.LoginRequest{
 		SetupKey:            providedSetupKey,
 		ManagementUrl:       managementURL,
-		IsUnixDesktopClient: util.HasGraphicalSession(),
+		IsUnixDesktopClient: isUnixRunningDesktop(),
 		Hostname:            hostName,
 		DnsLabels:           dnsLabelsReq,
 		ProfileName:         &handle,
@@ -188,8 +189,7 @@ func doExtendSession(ctx context.Context, cmd *cobra.Command) error {
 
 	client := proto.NewDaemonServiceClient(conn)
 
-	// the CLI runs in the user's session, the daemon does not: tell it what we can see
-	req := &proto.RequestExtendAuthSessionRequest{HasGraphicalSession: util.HasGraphicalSession()}
+	req := &proto.RequestExtendAuthSessionRequest{}
 	// Pre-fill the IdP login hint from the active profile so the user
 	// doesn't have to retype their email. Best-effort: we still proceed
 	// without a hint if the lookup fails.
@@ -408,9 +408,9 @@ func foregroundGetTokenInfo(ctx context.Context, cmd *cobra.Command, config *pro
 		hint = profileState.Email
 	}
 
-	oAuthFlow, err := auth.NewOAuthFlow(ctx, config, util.HasGraphicalSession(), false, hint)
+	oAuthFlow, err := auth.NewOAuthFlow(ctx, config, isUnixRunningDesktop(), false, hint)
 	if err != nil {
-		return nil, auth.WithSetupKeyAdvice(err)
+		return nil, err
 	}
 
 	flowInfo, err := oAuthFlow.RequestAuthInfo(context.TODO())
@@ -458,6 +458,14 @@ func openURL(cmd *cobra.Command, verificationURIComplete, userCode string, noBro
 	}
 }
 
+// isUnixRunningDesktop checks if a Linux OS is running desktop environment
+func isUnixRunningDesktop() bool {
+	if runtime.GOOS != "linux" && runtime.GOOS != "freebsd" {
+		return false
+	}
+	return os.Getenv("DESKTOP_SESSION") != "" || os.Getenv("XDG_CURRENT_DESKTOP") != ""
+}
+
 func setEnvAndFlags(cmd *cobra.Command) error {
 	SetFlagsFromEnvVars(rootCmd)
 

client/cmd/up.go

@@ -21,8 +21,8 @@ import (
 	"github.com/netbirdio/netbird/client/internal"
 	"github.com/netbirdio/netbird/client/internal/peer"
 	"github.com/netbirdio/netbird/client/internal/profilemanager"
-	nbnet "github.com/netbirdio/netbird/client/net"
 	"github.com/netbirdio/netbird/client/proto"
+	nbnet "github.com/netbirdio/netbird/client/net"
 	"github.com/netbirdio/netbird/client/server"
 	"github.com/netbirdio/netbird/client/system"
 	"github.com/netbirdio/netbird/shared/management/domain"
@@ -626,7 +626,7 @@ func setupLoginRequest(providedSetupKey string, customDNSAddressConverted []byte
 		NatExternalIPs:      natExternalIPs,
 		CleanNATExternalIPs: natExternalIPs != nil && len(natExternalIPs) == 0,
 		CustomDNSAddress:    customDNSAddressConverted,
-		IsUnixDesktopClient: util.HasGraphicalSession(),
+		IsUnixDesktopClient: isUnixRunningDesktop(),
 		Hostname:            hostName,
 		ExtraIFaceBlacklist: extraIFaceBlackList,
 		DnsLabels:           dnsLabels,

client/internal/auth/auth.go

@@ -83,15 +83,6 @@ func NewAuth(ctx context.Context, privateKey string, mgmURL *url.URL, config *pr
 	}, nil
 }
 
-// grpcClient returns the current management connection. Callers must go through it rather than
-// reading a.client: reconnect replaces that field while other goroutines are using it.
-func (a *Auth) grpcClient() *mgm.GrpcClient {
-	a.mutex.RLock()
-	defer a.mutex.RUnlock()
-
-	return a.client
-}
-
 // Close closes the management client connection
 func (a *Auth) Close() error {
 	a.mutex.Lock()
@@ -149,20 +140,25 @@ func (a *Auth) IsSSOSupported(ctx context.Context) (bool, error) {
 // This avoids creating a new connection to the management server
 func (a *Auth) GetOAuthFlow(ctx context.Context, forceDeviceAuth bool) (OAuthFlow, error) {
 	var flow OAuthFlow
+	var err error
 
-	// the connection is owned by a and outlives this call, so a later fallback reuses it
-	newAuth := func(context.Context) (*Auth, func(), error) {
-		return a, func() {}, nil
-	}
-
-	err := a.withRetry(ctx, func(client *mgm.GrpcClient) error {
-		var err error
-		flow, err = oauthFlowWithFallback(a, client, flowOrder(forceDeviceAuth, true), "", newAuth)
-
-		if IsSSOUnavailable(err) {
-			return backoff.Permanent(err)
+	err = a.withRetry(ctx, func(client *mgm.GrpcClient) error {
+		if forceDeviceAuth {
+			flow, err = a.getDeviceFlow(client)
+			return err
 		}
-		return err
+
+		// Try PKCE flow first
+		flow, err = a.getPKCEFlow(client)
+		if err != nil {
+			// If PKCE not supported, try Device flow
+			if s, ok := status.FromError(err); ok && (s.Code() == codes.NotFound || s.Code() == codes.Unimplemented) {
+				flow, err = a.getDeviceFlow(client)
+				return err
+			}
+			return err
+		}
+		return nil
 	})
 
 	return flow, err

client/internal/auth/device_flow.go

@@ -48,17 +48,8 @@ type DeviceAuthProviderConfig struct {
 	LoginHint string
 }
 
-// validateDeviceAuthConfig validates device authorization provider configuration. A missing
-// value means management does not have this flow configured, so the error wraps
-// errFlowNotConfigured and the caller can fall back to the other flow.
+// validateDeviceAuthConfig validates device authorization provider configuration
 func validateDeviceAuthConfig(config *DeviceAuthProviderConfig) error {
-	if err := checkDeviceAuthConfig(config); err != nil {
-		return fmt.Errorf("%w: %w", errFlowNotConfigured, err)
-	}
-	return nil
-}
-
-func checkDeviceAuthConfig(config *DeviceAuthProviderConfig) error {
 	errorMsgFormat := "invalid provider configuration received from management: %s value is empty. Contact your NetBird administrator"
 
 	if config.Audience == "" {
@@ -170,12 +161,8 @@ func (d *DeviceAuthorizationFlow) RequestAuthInfo(ctx context.Context) (AuthFlow
 		return AuthFlowInfo{}, fmt.Errorf("reading body failed with error: %v", err)
 	}
 
-	if res.StatusCode != http.StatusOK {
-		reqErr := fmt.Errorf("request device code returned status %d error: %s", res.StatusCode, string(body))
-		if deviceGrantUnsupported(res.StatusCode, body) {
-			return AuthFlowInfo{}, fmt.Errorf("%w: %w", errFlowNotConfigured, reqErr)
-		}
-		return AuthFlowInfo{}, reqErr
+	if res.StatusCode != 200 {
+		return AuthFlowInfo{}, fmt.Errorf("request device code returned status %d error: %s", res.StatusCode, string(body))
 	}
 
 	deviceCode := AuthFlowInfo{}
@@ -199,34 +186,6 @@ func (d *DeviceAuthorizationFlow) RequestAuthInfo(ctx context.Context) (AuthFlow
 	return deviceCode, err
 }
 
-// deviceGrantUnsupported reports whether the IdP's answer to a device code request means it does
-// not serve the device authorization grant at all, rather than a transient or request-specific
-// failure. An IdP that does not route the endpoint answers 404/405/501; one that knows the
-// endpoint but has the grant disabled for this client answers with an OAuth 2.0 error code.
-func deviceGrantUnsupported(statusCode int, body []byte) bool {
-	switch statusCode {
-	case http.StatusNotFound, http.StatusMethodNotAllowed, http.StatusNotImplemented:
-		return true
-	case http.StatusBadRequest, http.StatusUnauthorized, http.StatusForbidden:
-	default:
-		return false
-	}
-
-	var oauthErr struct {
-		Error string `json:"error"`
-	}
-	if err := json.Unmarshal(body, &oauthErr); err != nil {
-		return false
-	}
-
-	switch oauthErr.Error {
-	case "unsupported_grant_type", "unauthorized_client":
-		return true
-	default:
-		return false
-	}
-}
-
 func appendLoginHint(uri, loginHint string) string {
 	if uri == "" || loginHint == "" {
 		return uri

client/internal/auth/oauth.go

@@ -2,19 +2,15 @@ package auth
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"net/http"
-	"net/url"
 	"runtime"
-	"sync"
 
 	log "github.com/sirupsen/logrus"
 	"google.golang.org/grpc/codes"
 	gstatus "google.golang.org/grpc/status"
 
 	"github.com/netbirdio/netbird/client/internal/profilemanager"
-	mgm "github.com/netbirdio/netbird/shared/management/client"
 )
 
 // OAuthFlow represents an interface for authorization using different OAuth 2.0 flows
@@ -63,327 +59,77 @@ func (t TokenInfo) GetTokenToUse() string {
 	return t.AccessToken
 }
 
-// errFlowNotConfigured marks a flow this deployment does not offer: management returned no
-// configuration for it, the configuration it returned is incomplete, or the IdP refuses to serve
-// the grant. It is the only condition that makes the client try the other flow, so that a
-// transient failure keeps failing on the flow the user actually wants.
-var errFlowNotConfigured = errors.New("authorization flow is not configured")
-
-// ssoUnavailableError reports that the management server offers no usable SSO flow at all.
-// Retrying cannot help, so callers should surface it to the user instead of backing off.
-type ssoUnavailableError struct {
-	msg string
+func shouldUseDeviceFlow(force bool, isUnixDesktopClient bool) bool {
+	return force || (runtime.GOOS == "linux" || runtime.GOOS == "freebsd") && !isUnixDesktopClient
 }
 
-func (e *ssoUnavailableError) Error() string {
-	return e.msg
-}
-
-// oauthFlowInit names one of the OAuth flows and builds it from the management configuration.
-type oauthFlowInit struct {
-	name string
-	init func(a *Auth, client *mgm.GrpcClient, hint string) (OAuthFlow, error)
-}
-
-// authFactory hands out a management connection to build a flow with, plus the cleanup that
-// releases it. Callers that own a long-lived connection return it with a no-op cleanup.
-type authFactory func(ctx context.Context) (*Auth, func(), error)
-
-// loginHintSetter is implemented by both concrete flows but is deliberately not part of
-// OAuthFlow, so callers reach it through a type assertion.
-type loginHintSetter interface {
-	SetLoginHint(hint string)
-}
-
-// fallbackFlow wraps the flow that was picked at initialization time with the flows that were
-// not tried. Whether the IdP actually serves a flow only shows up when the flow is run: an IdP
-// with the device grant disabled answers the device code request with 404 even though
-// management handed out a device flow configuration. When that happens the wrapper swaps in the
-// next flow instead of failing the login.
-type fallbackFlow struct {
-	mu        sync.Mutex
-	active    OAuthFlow
-	remaining []oauthFlowInit
-	hint      string
-	newAuth   authFactory
-}
-
-func (f *fallbackFlow) RequestAuthInfo(ctx context.Context) (AuthFlowInfo, error) {
-	info, err := f.current().RequestAuthInfo(ctx)
-	if err == nil || !isFlowUnavailable(err) {
-		return info, err
-	}
-
-	next, nextErr := f.initNext(ctx)
-	if nextErr != nil {
-		log.Debugf("failed to fall back to another authorization flow: %v", nextErr)
-		return AuthFlowInfo{}, err
-	}
-
-	return next.RequestAuthInfo(ctx)
-}
-
-func (f *fallbackFlow) WaitToken(ctx context.Context, info AuthFlowInfo) (TokenInfo, error) {
-	return f.current().WaitToken(ctx, info)
-}
-
-func (f *fallbackFlow) GetClientID(ctx context.Context) string {
-	return f.current().GetClientID(ctx)
-}
-
-// SetLoginHint forwards the hint to the active flow and keeps it for a flow a later fallback
-// initializes. Callers that set the hint after building the flow reach the concrete flow through
-// a type assertion, which the OAuthFlow interface does not carry, so the wrapper has to offer it
-// too or the hint is silently dropped.
-func (f *fallbackFlow) SetLoginHint(hint string) {
-	f.mu.Lock()
-	defer f.mu.Unlock()
-
-	f.hint = hint
-	if setter, ok := f.active.(loginHintSetter); ok {
-		setter.SetLoginHint(hint)
-	}
-}
-
-func (f *fallbackFlow) current() OAuthFlow {
-	f.mu.Lock()
-	defer f.mu.Unlock()
-
-	return f.active
-}
-
-// initNext initializes the next flow this deployment offers and makes it the active one.
-func (f *fallbackFlow) initNext(ctx context.Context) (OAuthFlow, error) {
-	f.mu.Lock()
-	defer f.mu.Unlock()
-
-	if len(f.remaining) == 0 {
-		return nil, errors.New("no authorization flow left to try")
-	}
-
-	a, cleanup, err := f.newAuth(ctx)
-	if err != nil {
-		return nil, err
-	}
-	defer cleanup()
-
-	flow, remaining, err := initFirstAvailableFlow(a, a.grpcClient(), f.remaining, f.hint)
-	if err != nil {
-		return nil, err
-	}
-
-	log.Infof("the identity provider does not serve the selected authorization flow, continuing with the next one")
-	f.active = flow
-	f.remaining = remaining
-
-	return flow, nil
-}
-
-// preferDeviceFlow reports whether the device code flow should be tried before PKCE. PKCE needs
-// a browser on this host and a loopback listener to receive the redirect, neither of which
-// exists on a Unix host without a graphical session. The GOOS guard keeps a caller that reports
-// no graphical session on a platform that always has one from changing the preference.
-func preferDeviceFlow(hasGraphicalSession bool) bool {
-	return (runtime.GOOS == "linux" || runtime.GOOS == "freebsd") && !hasGraphicalSession
-}
-
-// flowOrder returns the flows to attempt, in order.
+// NewOAuthFlow initializes and returns the appropriate OAuth flow based on the management configuration
 //
-// force leaves the device code flow on its own rather than first: it marks a device with no
-// browser at all, such as Android TV or tvOS. PKCE cannot work there even from another device,
-// because the redirect has to arrive on the loopback listener of the device being enrolled, so
-// offering it as a fallback would only replace a clear error with a login that cannot complete.
-func flowOrder(force bool, hasGraphicalSession bool) []oauthFlowInit {
-	pkce := oauthFlowInit{name: "pkce authorization flow", init: initPKCEFlow}
-	device := oauthFlowInit{name: "device code flow", init: initDeviceFlow}
-
-	switch {
-	case force:
-		return []oauthFlowInit{device}
-	case preferDeviceFlow(hasGraphicalSession):
-		return []oauthFlowInit{device, pkce}
-	default:
-		return []oauthFlowInit{pkce, device}
-	}
-}
-
-func initPKCEFlow(a *Auth, client *mgm.GrpcClient, hint string) (OAuthFlow, error) {
-	flow, err := a.getPKCEFlow(client)
-	if err != nil {
-		return nil, err
-	}
-
-	if hint != "" {
-		flow.SetLoginHint(hint)
-	}
-
-	return flow, nil
-}
-
-func initDeviceFlow(a *Auth, client *mgm.GrpcClient, hint string) (OAuthFlow, error) {
-	flow, err := a.getDeviceFlow(client)
-	if err != nil {
-		return nil, err
-	}
-
-	if hint != "" {
-		flow.SetLoginHint(hint)
-	}
-
-	return flow, nil
-}
-
-// NewOAuthFlow initializes and returns an OAuth flow based on the management configuration.
+// It starts by initializing the PKCE.If this process fails, it resorts to the Device Code Flow,
+// and if that also fails, the authentication process is deemed unsuccessful
 //
-// Both flows are optional server side: management answers NotFound for a flow it has no
-// configuration for. The preferred flow is tried first and the other one is used as a fallback,
-// so a server that only offers one of them still works. forceDeviceCodeFlow restricts the client
-// to the device code flow with no fallback, for a device that has no browser at all.
-func NewOAuthFlow(ctx context.Context, config *profilemanager.Config, hasGraphicalSession bool, forceDeviceCodeFlow bool, hint string) (OAuthFlow, error) {
+// On Linux distros without desktop environment support, it only tries to initialize the Device Code Flow
+// forceDeviceCodeFlow can be used to skip PKCE and go directly to Device Code Flow (e.g., for Android TV)
+func NewOAuthFlow(ctx context.Context, config *profilemanager.Config, isUnixDesktopClient bool, forceDeviceCodeFlow bool, hint string) (OAuthFlow, error) {
+	if shouldUseDeviceFlow(forceDeviceCodeFlow, isUnixDesktopClient) {
+		return authenticateWithDeviceCodeFlow(ctx, config, hint)
+	}
+
+	pkceFlow, err := authenticateWithPKCEFlow(ctx, config, hint)
+	if err != nil {
+		log.Debugf("failed to initialize pkce authentication with error: %v\n", err)
+		log.Debug("falling back to device code flow")
+		return authenticateWithDeviceCodeFlow(ctx, config, hint)
+	}
+	return pkceFlow, nil
+}
+
+// authenticateWithPKCEFlow initializes the Proof Key for Code Exchange flow auth flow
+func authenticateWithPKCEFlow(ctx context.Context, config *profilemanager.Config, hint string) (OAuthFlow, error) {
 	authClient, err := NewAuth(ctx, config.PrivateKey, config.ManagementURL, config)
 	if err != nil {
-		return nil, fmt.Errorf("create auth client: %w", err)
+		return nil, fmt.Errorf("failed to create auth client: %v", err)
 	}
-	defer func() {
-		if err := authClient.Close(); err != nil {
-			log.Debugf("failed to close auth client: %v", err)
-		}
-	}()
+	defer authClient.Close()
 
-	// the connection above is closed on return, so a later fallback opens its own
-	newAuth := func(ctx context.Context) (*Auth, func(), error) {
-		a, err := NewAuth(ctx, config.PrivateKey, config.ManagementURL, config)
-		if err != nil {
-			return nil, nil, fmt.Errorf("create auth client: %w", err)
-		}
-		return a, func() {
-			if err := a.Close(); err != nil {
-				log.Debugf("failed to close auth client: %v", err)
-			}
-		}, nil
-	}
-
-	flows := flowOrder(forceDeviceCodeFlow, hasGraphicalSession)
-	return oauthFlowWithFallback(authClient, authClient.grpcClient(), flows, hint, newAuth)
-}
-
-// oauthFlowWithFallback initializes the first flow this deployment offers, moving on to the next
-// one when a flow is not configured here. It only fails once every flow has been tried, and any
-// flow left untried is handed to the returned flow so it can still fall back if the IdP rejects
-// the flow that was picked.
-func oauthFlowWithFallback(a *Auth, client *mgm.GrpcClient, flows []oauthFlowInit, hint string, newAuth authFactory) (OAuthFlow, error) {
-	flow, remaining, err := initFirstAvailableFlow(a, client, flows, hint)
+	pkceFlowInfo, err := authClient.getPKCEFlow(authClient.client)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("getting pkce authorization flow info failed with error: %v", err)
 	}
 
-	if len(remaining) == 0 {
-		return flow, nil
+	if hint != "" {
+		pkceFlowInfo.SetLoginHint(hint)
 	}
 
-	return &fallbackFlow{
-		active:    flow,
-		remaining: remaining,
-		hint:      hint,
-		newAuth:   newAuth,
-	}, nil
+	return pkceFlowInfo, nil
 }
 
-// initFirstAvailableFlow returns the first flow that could be initialized along with the flows
-// after it, which are still untried.
-func initFirstAvailableFlow(a *Auth, client *mgm.GrpcClient, flows []oauthFlowInit, hint string) (OAuthFlow, []oauthFlowInit, error) {
-	var errs []error
-	for i, f := range flows {
-		flow, err := f.init(a, client, hint)
-		if err == nil {
-			return flow, flows[i+1:], nil
-		}
+// authenticateWithDeviceCodeFlow initializes the Device Code auth Flow
+func authenticateWithDeviceCodeFlow(ctx context.Context, config *profilemanager.Config, hint string) (OAuthFlow, error) {
+	authClient, err := NewAuth(ctx, config.PrivateKey, config.ManagementURL, config)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create auth client: %v", err)
+	}
+	defer authClient.Close()
 
-		errs = append(errs, fmt.Errorf("%s: %w", f.name, err))
-
-		// only a flow this deployment does not offer is worth replacing with another one
-		if !isFlowUnavailable(err) {
-			break
-		}
-		if i < len(flows)-1 {
-			log.Infof("%s is not configured (%v), falling back to %s", f.name, err, flows[i+1].name)
+	deviceFlowInfo, err := authClient.getDeviceFlow(authClient.client)
+	if err != nil {
+		switch s, ok := gstatus.FromError(err); {
+		case ok && s.Code() == codes.NotFound:
+			return nil, fmt.Errorf("no SSO provider returned from management. " +
+				"Please proceed with setting up this device using setup keys " +
+				"https://docs.netbird.io/how-to/register-machines-using-setup-keys")
+		case ok && s.Code() == codes.Unimplemented:
+			return nil, fmt.Errorf("the management server, %s, does not support SSO providers, "+
+				"please update your server or use Setup Keys to login", config.ManagementURL)
+		default:
+			return nil, fmt.Errorf("getting device authorization flow info failed with error: %v", err)
 		}
 	}
 
-	return nil, nil, flowInitError(a.mgmURL, errs)
-}
-
-// flowInitError turns the per-flow initialization errors into a single actionable error. The
-// message stays neutral about what to do instead: SSO is also how a peer extends its session and
-// authenticates SSH, where a setup key is no alternative. Callers that are enrolling a device add
-// that advice themselves, see IsSSOUnavailable.
-func flowInitError(mgmURL *url.URL, errs []error) error {
-	if allMatch(errs, isFlowUnimplemented) {
-		return &ssoUnavailableError{msg: fmt.Sprintf("the management server, %s, does not support SSO providers, "+
-			"please update your server", mgmURL)}
+	if hint != "" {
+		deviceFlowInfo.SetLoginHint(hint)
 	}
 
-	if allMatch(errs, isFlowUnavailable) {
-		return &ssoUnavailableError{msg: "the management server has no SSO provider configured: " +
-			"neither the pkce authorization flow nor the device code flow is available"}
-	}
-
-	return fmt.Errorf("initialize authorization flow: %w", errors.Join(errs...))
-}
-
-// IsSSOUnavailable reports whether err means the management server offers no usable SSO flow, so
-// no retry and no other flow can help. Enrollment paths use it to point the user at setup keys.
-func IsSSOUnavailable(err error) bool {
-	var ssoUnavailable *ssoUnavailableError
-	return errors.As(err, &ssoUnavailable)
-}
-
-// WithSetupKeyAdvice appends enrollment guidance to an SSO-unavailable error and returns any
-// other error unchanged. Only enrollment can fall back to a setup key: extending a session and
-// authenticating SSH cannot, so those paths must not call this.
-//
-// The login paths that do call it cannot tell an unregistered peer from an SSO-enrolled one
-// whose session expired, since both answer PermissionDenied, so the advice names the case it
-// applies to rather than telling an enrolled peer to do something that cannot work.
-func WithSetupKeyAdvice(err error) error {
-	if !IsSSOUnavailable(err) {
-		return err
-	}
-
-	return fmt.Errorf("%w. If this device is not enrolled yet, enroll it with a setup key instead: "+
-		"https://docs.netbird.io/how-to/register-machines-using-setup-keys", err)
-}
-
-func allMatch(errs []error, match func(error) bool) bool {
-	if len(errs) == 0 {
-		return false
-	}
-
-	for _, err := range errs {
-		if !match(err) {
-			return false
-		}
-	}
-	return true
-}
-
-// isFlowUnavailable reports whether the flow is not on offer here: management has no
-// configuration for it (NotFound), predates the RPC entirely (Unimplemented), returned an
-// incomplete configuration, or the IdP does not serve the grant.
-func isFlowUnavailable(err error) bool {
-	return errors.Is(err, errFlowNotConfigured) ||
-		hasStatusCode(err, codes.NotFound) ||
-		hasStatusCode(err, codes.Unimplemented)
-}
-
-func isFlowUnimplemented(err error) bool {
-	return hasStatusCode(err, codes.Unimplemented)
-}
-
-func hasStatusCode(err error, code codes.Code) bool {
-	s, ok := gstatus.FromError(err)
-	if !ok {
-		return false
-	}
-	return s.Code() == code
+	return deviceFlowInfo, nil
 }

client/internal/auth/oauth_test.go

@@ -1,330 +0,0 @@
-package auth
-
-import (
-	"context"
-	"errors"
-	"fmt"
-	"net/url"
-	"runtime"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
-
-	mgm "github.com/netbirdio/netbird/shared/management/client"
-)
-
-// stubFlow is a minimal OAuthFlow returned by the fake initializers below. requestErr, when set,
-// is what its RequestAuthInfo returns, standing in for an IdP that rejects the flow.
-type stubFlow struct {
-	name       string
-	hint       string
-	requestErr error
-}
-
-func (s *stubFlow) RequestAuthInfo(context.Context) (AuthFlowInfo, error) {
-	if s.requestErr != nil {
-		return AuthFlowInfo{}, s.requestErr
-	}
-	return AuthFlowInfo{UserCode: s.name}, nil
-}
-
-func (s *stubFlow) WaitToken(context.Context, AuthFlowInfo) (TokenInfo, error) {
-	return TokenInfo{}, nil
-}
-
-func (s *stubFlow) GetClientID(context.Context) string {
-	return ""
-}
-
-func (s *stubFlow) SetLoginHint(hint string) {
-	s.hint = hint
-}
-
-// stubInit returns a flow initializer that yields a named stub flow, or err when err is non-nil.
-func stubInit(name string, err error) oauthFlowInit {
-	return stubInitFlow(name, err, nil)
-}
-
-// stubInitFlow is stubInit with control over what the resulting flow's RequestAuthInfo returns.
-func stubInitFlow(name string, initErr, requestErr error) oauthFlowInit {
-	return oauthFlowInit{
-		name: name,
-		init: func(_ *Auth, _ *mgm.GrpcClient, hint string) (OAuthFlow, error) {
-			if initErr != nil {
-				return nil, initErr
-			}
-			return &stubFlow{name: name, hint: hint, requestErr: requestErr}, nil
-		},
-	}
-}
-
-// stubAuthFactory hands out an Auth without a management connection, which the stub
-// initializers above never touch.
-func stubAuthFactory(a *Auth) authFactory {
-	return func(context.Context) (*Auth, func(), error) {
-		return a, func() {}, nil
-	}
-}
-
-func TestOAuthFlowWithFallback(t *testing.T) {
-	notFound := status.Error(codes.NotFound, "no device authorization flow information available")
-	unimplemented := status.Error(codes.Unimplemented, "unknown method")
-	incompleteConfig := fmt.Errorf("%w: Client ID value is empty", errFlowNotConfigured)
-	unreachable := status.Error(codes.Unavailable, "connection refused")
-
-	tests := []struct {
-		name          string
-		flows         []oauthFlowInit
-		expectedFlow  string
-		expectedErr   string
-		expectedNoSSO bool
-	}{
-		{
-			name:         "preferred flow is used",
-			flows:        []oauthFlowInit{stubInit("device", nil), stubInit("pkce", nil)},
-			expectedFlow: "device",
-		},
-		{
-			// the RedHat case: device code flow disabled on management, PKCE configured
-			name:         "falls back when preferred flow is not configured",
-			flows:        []oauthFlowInit{stubInit("device", notFound), stubInit("pkce", nil)},
-			expectedFlow: "pkce",
-		},
-		{
-			name:         "falls back on an incomplete flow configuration",
-			flows:        []oauthFlowInit{stubInit("pkce", incompleteConfig), stubInit("device", nil)},
-			expectedFlow: "device",
-		},
-		{
-			name:        "does not fall back when the preferred flow fails for another reason",
-			flows:       []oauthFlowInit{stubInit("pkce", unreachable), stubInit("device", nil)},
-			expectedErr: "connection refused",
-		},
-		{
-			// stays neutral about the remedy: --extend and SSH auth cannot use a setup key
-			name:          "neither flow configured reports no SSO provider",
-			flows:         []oauthFlowInit{stubInit("device", notFound), stubInit("pkce", notFound)},
-			expectedErr:   "no SSO provider configured",
-			expectedNoSSO: true,
-		},
-		{
-			name:          "old server without the flow RPCs asks for an update",
-			flows:         []oauthFlowInit{stubInit("device", unimplemented), stubInit("pkce", unimplemented)},
-			expectedErr:   "does not support SSO providers",
-			expectedNoSSO: true,
-		},
-	}
-
-	mgmURL, err := url.Parse("https://api.netbird.io:443")
-	require.NoError(t, err)
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			a := &Auth{mgmURL: mgmURL}
-			flow, err := oauthFlowWithFallback(a, nil, tt.flows, "user@example.com", stubAuthFactory(a))
-
-			if tt.expectedErr != "" {
-				require.Error(t, err)
-				assert.Contains(t, err.Error(), tt.expectedErr)
-
-				var ssoUnavailable *ssoUnavailableError
-				assert.Equal(t, tt.expectedNoSSO, errors.As(err, &ssoUnavailable),
-					"terminal SSO-unavailable classification mismatch for %v", err)
-				return
-			}
-
-			require.NoError(t, err)
-			stub := activeStub(t, flow)
-			assert.Equal(t, tt.expectedFlow, stub.name)
-			assert.Equal(t, "user@example.com", stub.hint, "login hint must be passed to the flow")
-		})
-	}
-}
-
-// activeStub unwraps the flow currently in use, which is behind a fallbackFlow whenever an
-// untried flow is left.
-func activeStub(t *testing.T, flow OAuthFlow) *stubFlow {
-	t.Helper()
-
-	if fallback, ok := flow.(*fallbackFlow); ok {
-		flow = fallback.current()
-	}
-
-	stub, ok := flow.(*stubFlow)
-	require.True(t, ok, "unexpected flow type %T", flow)
-	return stub
-}
-
-// TestFallbackFlowRequestAuthInfo covers the failure the RedHat report hit: management hands out
-// a device flow configuration, but the IdP does not serve the grant and only says so when the
-// device code is requested.
-func TestFallbackFlowRequestAuthInfo(t *testing.T) {
-	mgmURL, err := url.Parse("https://api.netbird.io:443")
-	require.NoError(t, err)
-	a := &Auth{mgmURL: mgmURL}
-
-	idpRejects := fmt.Errorf("%w: request device code returned status 404", errFlowNotConfigured)
-
-	t.Run("swaps in the untried flow", func(t *testing.T) {
-		flows := []oauthFlowInit{stubInitFlow("device", nil, idpRejects), stubInit("pkce", nil)}
-
-		flow, err := oauthFlowWithFallback(a, nil, flows, "", stubAuthFactory(a))
-		require.NoError(t, err)
-		require.Equal(t, "device", activeStub(t, flow).name)
-
-		info, err := flow.RequestAuthInfo(context.Background())
-		require.NoError(t, err)
-		assert.Equal(t, "pkce", info.UserCode, "the request must be served by the fallback flow")
-		assert.Equal(t, "pkce", activeStub(t, flow).name, "the fallback flow must stay active for WaitToken")
-	})
-
-	t.Run("keeps the original error when nothing else is configured", func(t *testing.T) {
-		flows := []oauthFlowInit{
-			stubInitFlow("device", nil, idpRejects),
-			stubInit("pkce", status.Error(codes.NotFound, "no pkce authorization flow information available")),
-		}
-
-		flow, err := oauthFlowWithFallback(a, nil, flows, "", stubAuthFactory(a))
-		require.NoError(t, err)
-
-		_, err = flow.RequestAuthInfo(context.Background())
-		require.Error(t, err)
-		assert.Contains(t, err.Error(), "status 404")
-	})
-
-	t.Run("keeps the original error when the fallback cannot reach management", func(t *testing.T) {
-		flows := []oauthFlowInit{stubInitFlow("device", nil, idpRejects), stubInit("pkce", nil)}
-
-		unreachable := func(context.Context) (*Auth, func(), error) {
-			return nil, nil, errors.New("connect to management: connection refused")
-		}
-
-		flow, err := oauthFlowWithFallback(a, nil, flows, "", unreachable)
-		require.NoError(t, err)
-
-		_, err = flow.RequestAuthInfo(context.Background())
-		require.Error(t, err)
-		assert.Contains(t, err.Error(), "status 404", "the IdP error must survive a failed fallback")
-		assert.Equal(t, "device", activeStub(t, flow).name, "a failed fallback must not swap the flow")
-	})
-
-	t.Run("does not swap flows on an unrelated failure", func(t *testing.T) {
-		flows := []oauthFlowInit{
-			stubInitFlow("device", nil, errors.New("timeout talking to the IdP")),
-			stubInit("pkce", nil),
-		}
-
-		flow, err := oauthFlowWithFallback(a, nil, flows, "", stubAuthFactory(a))
-		require.NoError(t, err)
-
-		_, err = flow.RequestAuthInfo(context.Background())
-		require.Error(t, err)
-		assert.Equal(t, "device", activeStub(t, flow).name, "the preferred flow must stay active")
-	})
-}
-
-// TestForcedDeviceFlowHasNoFallback covers Android TV and tvOS: a browserless device must get the
-// device code error rather than a PKCE flow it can never complete.
-func TestForcedDeviceFlowHasNoFallback(t *testing.T) {
-	mgmURL, err := url.Parse("https://api.netbird.io:443")
-	require.NoError(t, err)
-	a := &Auth{mgmURL: mgmURL}
-
-	notFound := status.Error(codes.NotFound, "no device authorization flow information available")
-
-	t.Run("no wrapper when the device flow works", func(t *testing.T) {
-		// flowOrder(force) yields this single-entry list, see TestFlowOrder
-		forced := []oauthFlowInit{stubInit("device", nil)}
-
-		flow, err := oauthFlowWithFallback(a, nil, forced, "", stubAuthFactory(a))
-		require.NoError(t, err)
-
-		_, wrapped := flow.(*fallbackFlow)
-		assert.False(t, wrapped, "nothing may swap the flow later on a browserless device")
-	})
-
-	t.Run("reports the device flow error instead of falling back", func(t *testing.T) {
-		forced := []oauthFlowInit{stubInit("device", notFound)}
-
-		_, err := oauthFlowWithFallback(a, nil, forced, "", stubAuthFactory(a))
-		require.Error(t, err)
-		assert.True(t, IsSSOUnavailable(err), "the caller must see that SSO is unavailable here")
-	})
-}
-
-// TestFallbackFlowSetLoginHint covers the Android SDK's pattern: it sets the login hint after the
-// flow is built, through a type assertion that the wrapper must satisfy.
-func TestFallbackFlowSetLoginHint(t *testing.T) {
-	mgmURL, err := url.Parse("https://api.netbird.io:443")
-	require.NoError(t, err)
-	a := &Auth{mgmURL: mgmURL}
-
-	idpRejects := fmt.Errorf("%w: request device code returned status 404", errFlowNotConfigured)
-	flows := []oauthFlowInit{stubInitFlow("device", nil, idpRejects), stubInit("pkce", nil)}
-
-	flow, err := oauthFlowWithFallback(a, nil, flows, "", stubAuthFactory(a))
-	require.NoError(t, err)
-
-	setter, ok := flow.(loginHintSetter)
-	require.True(t, ok, "the wrapper must accept a login hint like the concrete flows do")
-	setter.SetLoginHint("user@example.com")
-	assert.Equal(t, "user@example.com", activeStub(t, flow).hint, "the active flow must get the hint")
-
-	// the device flow is rejected by the IdP here, so the hint has to survive into the fallback
-	_, err = flow.RequestAuthInfo(context.Background())
-	require.NoError(t, err)
-	assert.Equal(t, "pkce", activeStub(t, flow).name)
-	assert.Equal(t, "user@example.com", activeStub(t, flow).hint, "the fallback flow must get the hint too")
-}
-
-func TestWithSetupKeyAdvice(t *testing.T) {
-	other := errors.New("connection refused")
-	assert.Equal(t, other, WithSetupKeyAdvice(other), "only an SSO-unavailable error gets advice")
-
-	advised := WithSetupKeyAdvice(&ssoUnavailableError{msg: "no SSO provider configured"})
-	assert.Contains(t, advised.Error(), "no SSO provider configured", "the original message must survive")
-	assert.Contains(t, advised.Error(), "setup key")
-	// a setup key cannot re-enrol a peer whose SSO session expired, and the login paths cannot
-	// tell that peer apart from an unregistered one, so the advice must state its condition
-	assert.Contains(t, advised.Error(), "not enrolled yet")
-	assert.True(t, IsSSOUnavailable(advised), "advice must keep the error classifiable")
-}
-
-func flowNames(flows []oauthFlowInit) []string {
-	names := make([]string, 0, len(flows))
-	for _, f := range flows {
-		names = append(names, f.name)
-	}
-	return names
-}
-
-func TestFlowOrder(t *testing.T) {
-	const pkce, device = "pkce authorization flow", "device code flow"
-
-	assert.Equal(t, []string{pkce, device}, flowNames(flowOrder(false, true)),
-		"a device with a browser tries PKCE first and keeps the device code flow as a fallback")
-
-	// only a unix host without a graphical session lacks a browser; the other platforms have one
-	headless := []string{pkce, device}
-	if runtime.GOOS == "linux" || runtime.GOOS == "freebsd" {
-		headless = []string{device, pkce}
-	}
-	assert.Equal(t, headless, flowNames(flowOrder(false, false)), "on %s", runtime.GOOS)
-
-	// Android TV and tvOS have no browser, so PKCE cannot complete there even from another
-	// device: the redirect must reach the loopback listener of the device being enrolled.
-	assert.Equal(t, []string{device}, flowNames(flowOrder(true, false)),
-		"a forced device code flow must not fall back to PKCE")
-	assert.Equal(t, []string{device}, flowNames(flowOrder(true, true)),
-		"force wins over a reported graphical session")
-}
-
-func TestPreferDeviceFlow(t *testing.T) {
-	isUnix := runtime.GOOS == "linux" || runtime.GOOS == "freebsd"
-
-	assert.Equal(t, isUnix, preferDeviceFlow(false), "headless unix hosts prefer the device flow")
-	assert.False(t, preferDeviceFlow(true), "clients with a graphical session prefer PKCE")
-}

client/internal/auth/pkce_flow.go

@@ -62,17 +62,8 @@ type PKCEAuthProviderConfig struct {
 	LoginHint string
 }
 
-// validatePKCEConfig validates PKCE provider configuration. A missing value means management
-// does not have this flow configured, so the error wraps errFlowNotConfigured and the caller can
-// fall back to the other flow.
+// validatePKCEConfig validates PKCE provider configuration
 func validatePKCEConfig(config *PKCEAuthProviderConfig) error {
-	if err := checkPKCEConfig(config); err != nil {
-		return fmt.Errorf("%w: %w", errFlowNotConfigured, err)
-	}
-	return nil
-}
-
-func checkPKCEConfig(config *PKCEAuthProviderConfig) error {
 	errorMsgFormat := "invalid provider configuration received from management: %s value is empty. Contact your NetBird administrator"
 
 	if config.ClientID == "" {

client/proto/daemon.pb.go

@@ -5628,13 +5628,9 @@ func (x *GetPeerSSHHostKeyResponse) GetFound() bool {
 type RequestJWTAuthRequest struct {
 	state protoimpl.MessageState `protogen:"open.v1"`
 	// hint for OIDC login_hint parameter (typically email address)
-	Hint *string `protobuf:"bytes,1,opt,name=hint,proto3,oneof" json:"hint,omitempty"`
-	// hasGraphicalSession tells the daemon that the caller has a graphical session,
-	// which decides whether PKCE or the device code flow is preferred. The daemon
-	// cannot detect this itself: it does not inherit the session environment.
-	HasGraphicalSession bool `protobuf:"varint,2,opt,name=hasGraphicalSession,proto3" json:"hasGraphicalSession,omitempty"`
-	unknownFields       protoimpl.UnknownFields
-	sizeCache           protoimpl.SizeCache
+	Hint          *string `protobuf:"bytes,1,opt,name=hint,proto3,oneof" json:"hint,omitempty"`
+	unknownFields protoimpl.UnknownFields
+	sizeCache     protoimpl.SizeCache
 }
 
 func (x *RequestJWTAuthRequest) Reset() {
@@ -5674,13 +5670,6 @@ func (x *RequestJWTAuthRequest) GetHint() string {
 	return ""
 }
 
-func (x *RequestJWTAuthRequest) GetHasGraphicalSession() bool {
-	if x != nil {
-		return x.HasGraphicalSession
-	}
-	return false
-}
-
 // RequestJWTAuthResponse contains authentication flow information
 type RequestJWTAuthResponse struct {
 	state protoimpl.MessageState `protogen:"open.v1"`
@@ -5905,13 +5894,9 @@ type RequestExtendAuthSessionRequest struct {
 	state protoimpl.MessageState `protogen:"open.v1"`
 	// Optional OIDC login_hint (typically the user's email) to pre-fill the
 	// IdP login form.
-	Hint *string `protobuf:"bytes,1,opt,name=hint,proto3,oneof" json:"hint,omitempty"`
-	// hasGraphicalSession tells the daemon that the caller has a graphical session,
-	// which decides whether PKCE or the device code flow is preferred. The daemon
-	// cannot detect this itself: it does not inherit the session environment.
-	HasGraphicalSession bool `protobuf:"varint,2,opt,name=hasGraphicalSession,proto3" json:"hasGraphicalSession,omitempty"`
-	unknownFields       protoimpl.UnknownFields
-	sizeCache           protoimpl.SizeCache
+	Hint          *string `protobuf:"bytes,1,opt,name=hint,proto3,oneof" json:"hint,omitempty"`
+	unknownFields protoimpl.UnknownFields
+	sizeCache     protoimpl.SizeCache
 }
 
 func (x *RequestExtendAuthSessionRequest) Reset() {
@@ -5951,13 +5936,6 @@ func (x *RequestExtendAuthSessionRequest) GetHint() string {
 	return ""
 }
 
-func (x *RequestExtendAuthSessionRequest) GetHasGraphicalSession() bool {
-	if x != nil {
-		return x.HasGraphicalSession
-	}
-	return false
-}
-
 // RequestExtendAuthSessionResponse carries the verification URI the UI
 // should open in a browser. The daemon retains the flow state and resolves
 // it via WaitExtendAuthSession.
@@ -7525,10 +7503,9 @@ const file_daemon_proto_rawDesc = "" +
 	"sshHostKey\x12\x16\n" +
 	"\x06peerIP\x18\x02 \x01(\tR\x06peerIP\x12\x1a\n" +
 	"\bpeerFQDN\x18\x03 \x01(\tR\bpeerFQDN\x12\x14\n" +
-	"\x05found\x18\x04 \x01(\bR\x05found\"k\n" +
+	"\x05found\x18\x04 \x01(\bR\x05found\"9\n" +
 	"\x15RequestJWTAuthRequest\x12\x17\n" +
-	"\x04hint\x18\x01 \x01(\tH\x00R\x04hint\x88\x01\x01\x120\n" +
-	"\x13hasGraphicalSession\x18\x02 \x01(\bR\x13hasGraphicalSessionB\a\n" +
+	"\x04hint\x18\x01 \x01(\tH\x00R\x04hint\x88\x01\x01B\a\n" +
 	"\x05_hint\"\x9a\x02\n" +
 	"\x16RequestJWTAuthResponse\x12(\n" +
 	"\x0fverificationURI\x18\x01 \x01(\tR\x0fverificationURI\x128\n" +
@@ -7548,10 +7525,9 @@ const file_daemon_proto_rawDesc = "" +
 	"\x14WaitJWTTokenResponse\x12\x14\n" +
 	"\x05token\x18\x01 \x01(\tR\x05token\x12\x1c\n" +
 	"\ttokenType\x18\x02 \x01(\tR\ttokenType\x12\x1c\n" +
-	"\texpiresIn\x18\x03 \x01(\x03R\texpiresIn\"u\n" +
+	"\texpiresIn\x18\x03 \x01(\x03R\texpiresIn\"C\n" +
 	"\x1fRequestExtendAuthSessionRequest\x12\x17\n" +
-	"\x04hint\x18\x01 \x01(\tH\x00R\x04hint\x88\x01\x01\x120\n" +
-	"\x13hasGraphicalSession\x18\x02 \x01(\bR\x13hasGraphicalSessionB\a\n" +
+	"\x04hint\x18\x01 \x01(\tH\x00R\x04hint\x88\x01\x01B\a\n" +
 	"\x05_hint\"\xe0\x01\n" +
 	" RequestExtendAuthSessionResponse\x12(\n" +
 	"\x0fverificationURI\x18\x01 \x01(\tR\x0fverificationURI\x128\n" +

client/proto/daemon.proto

@@ -894,10 +894,6 @@ message GetPeerSSHHostKeyResponse {
 message RequestJWTAuthRequest {
   // hint for OIDC login_hint parameter (typically email address)
   optional string hint = 1;
-  // hasGraphicalSession tells the daemon that the caller has a graphical session,
-  // which decides whether PKCE or the device code flow is preferred. The daemon
-  // cannot detect this itself: it does not inherit the session environment.
-  bool hasGraphicalSession = 2;
 }
 
 // RequestJWTAuthResponse contains authentication flow information
@@ -941,10 +937,6 @@ message RequestExtendAuthSessionRequest {
   // Optional OIDC login_hint (typically the user's email) to pre-fill the
   // IdP login form.
   optional string hint = 1;
-  // hasGraphicalSession tells the daemon that the caller has a graphical session,
-  // which decides whether PKCE or the device code flow is preferred. The daemon
-  // cannot detect this itself: it does not inherit the session environment.
-  bool hasGraphicalSession = 2;
 }
 
 // RequestExtendAuthSessionResponse carries the verification URI the UI

client/server/server.go

@@ -682,11 +682,6 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro
 		oAuthFlow, err := auth.NewOAuthFlow(ctx, config, msg.IsUnixDesktopClient, false, hint)
 		if err != nil {
 			state.Set(internal.StatusLoginFailed)
-			// enrolling a device is the one flow a setup key can replace. NotFound so the CLI
-			// stops its backoff loop and shows this instead of retrying a permanent condition.
-			if auth.IsSSOUnavailable(err) {
-				return nil, gstatus.Error(codes.NotFound, auth.WithSetupKeyAdvice(err).Error())
-			}
 			return nil, err
 		}
 
@@ -1728,8 +1723,8 @@ func (s *Server) RequestJWTAuth(
 		hint = profilemanager.GetLoginHint()
 	}
 
-	// the daemon has no graphical session of its own, only the caller can answer this
-	oAuthFlow, err := auth.NewOAuthFlow(ctx, config, msg.GetHasGraphicalSession(), false, hint)
+	isDesktop := isUnixRunningDesktop()
+	oAuthFlow, err := auth.NewOAuthFlow(ctx, config, isDesktop, false, hint)
 	if err != nil {
 		return nil, gstatus.Errorf(codes.Internal, "failed to create OAuth flow: %v", err)
 	}
@@ -1832,8 +1827,8 @@ func (s *Server) RequestExtendAuthSession(
 		hint = profilemanager.GetLoginHint()
 	}
 
-	// the daemon has no graphical session of its own, only the caller can answer this
-	oAuthFlow, err := auth.NewOAuthFlow(ctx, config, msg.GetHasGraphicalSession(), false, hint)
+	isDesktop := isUnixRunningDesktop()
+	oAuthFlow, err := auth.NewOAuthFlow(ctx, config, isDesktop, false, hint)
 	if err != nil {
 		return nil, gstatus.Errorf(codes.Internal, "failed to create OAuth flow: %v", err)
 	}
@@ -2005,6 +2000,13 @@ func (s *Server) ExposeService(req *proto.ExposeServiceRequest, srv proto.Daemon
 	return nil
 }
 
+func isUnixRunningDesktop() bool {
+	if runtime.GOOS != "linux" && runtime.GOOS != "freebsd" {
+		return false
+	}
+	return os.Getenv("DESKTOP_SESSION") != "" || os.Getenv("XDG_CURRENT_DESKTOP") != ""
+}
+
 func (s *Server) runProbes(ctx context.Context, waitForProbeResult bool) {
 	if s.connectClient == nil {
 		return

client/ssh/common.go

@@ -13,7 +13,6 @@ import (
 	"golang.org/x/crypto/ssh"
 
 	"github.com/netbirdio/netbird/client/proto"
-	"github.com/netbirdio/netbird/util"
 )
 
 const (
@@ -93,8 +92,7 @@ func printAuthInstructions(stderr io.Writer, authResponse *proto.RequestJWTAuthR
 
 // RequestJWTToken requests or retrieves a JWT token for SSH authentication
 func RequestJWTToken(ctx context.Context, client proto.DaemonServiceClient, stdout, stderr io.Writer, useCache bool, hint string, openBrowser func(string) error) (string, error) {
-	// the ssh client runs in the user's session, the daemon does not: tell it what we can see
-	req := &proto.RequestJWTAuthRequest{HasGraphicalSession: util.HasGraphicalSession()}
+	req := &proto.RequestJWTAuthRequest{}
 	if hint != "" {
 		req.Hint = &hint
 	}
@@ -195,3 +193,4 @@ func buildAddressList(hostname string, remote net.Addr) []string {
 	}
 	return addresses
 }
+

client/ui/authsession/service.go

@@ -58,8 +58,7 @@ func (s *Session) RequestExtend(ctx context.Context, p ExtendStartParams) (Exten
 		return ExtendStartResult{}, err
 	}
 
-	// a request from the UI implies a graphical session, which the daemon cannot detect itself
-	req := &proto.RequestExtendAuthSessionRequest{HasGraphicalSession: true}
+	req := &proto.RequestExtendAuthSessionRequest{}
 	if p.Hint != "" {
 		h := p.Hint
 		req.Hint = &h

client/ui/services/connection.go

@@ -108,11 +108,10 @@ func (s *Connection) Login(ctx context.Context, p LoginParams) (LoginResult, err
 	}
 
 	req := &proto.LoginRequest{
-		ManagementUrl: p.ManagementURL,
-		SetupKey:      p.SetupKey,
-		Hostname:      p.Hostname,
-		// a login driven by the UI always has a graphical session available
-		IsUnixDesktopClient: true,
+		ManagementUrl:       p.ManagementURL,
+		SetupKey:            p.SetupKey,
+		Hostname:            p.Hostname,
+		IsUnixDesktopClient: runtime.GOOS == "linux",
 	}
 	if profileName != "" {
 		req.ProfileName = ptrStr(profileName)

infrastructure_files/getting-started-enterprise.sh

@@ -12,7 +12,10 @@ SED_STRIP_PADDING='s/=//g'
 NETBIRD_EULA_URL="https://netbird.io/self-hosted-EULA"
 
 # Static IP for Traefik inside the compose bridge network. The management
-# server trusts X-Forwarded-* headers from this address only.
+# server trusts X-Forwarded-* headers from this address only, so all three
+# values derive from the same /24. Override with NETBIRD_DOCKER_SUBNET.
+DOCKER_SUBNET="172.30.0.0/24"
+DOCKER_GATEWAY="172.30.0.1"
 TRAEFIK_IP="172.30.0.10"
 
 check_docker_compose() {
@@ -43,6 +46,127 @@ rand_b64_key() {
   openssl rand -base64 32
 }
 
+# ------------------------------------------------------------------
+# Docker network subnet override and conflict check
+# (kept in sync with getting-started.sh; only the compose network
+# name differs)
+# ------------------------------------------------------------------
+
+ip_to_int() {
+  local a b c d
+  IFS=. read -r a b c d <<< "$1"
+  echo $(( (10#$a << 24) + (10#$b << 16) + (10#$c << 8) + 10#$d ))
+}
+
+# cidrs_overlap <cidr> <cidr> — succeeds if the networks overlap
+cidrs_overlap() {
+  local net1="${1%/*}" len1="${1#*/}" net2="${2%/*}" len2="${2#*/}"
+  local min_len=$(( len1 < len2 ? len1 : len2 ))
+  local mask=0
+  if [[ "$min_len" -gt 0 ]]; then
+    mask=$(( (0xFFFFFFFF << (32 - min_len)) & 0xFFFFFFFF ))
+  fi
+  [[ $(( $(ip_to_int "$net1") & mask )) -eq $(( $(ip_to_int "$net2") & mask )) ]]
+}
+
+# valid_ipv4_slash24 <cidr> — accepts a unicast IPv4 /24 like 10.123.45.0/24
+valid_ipv4_slash24() {
+  local octet='(25[0-5]|2[0-4][0-9]|1[0-9][0-9]|[1-9][0-9]|[0-9])'
+  local re="^${octet}\.${octet}\.${octet}\.0/24$"
+  [[ "$1" =~ $re ]] || return 1
+  # Reject non-unicast/reserved ranges: 0/8, loopback, link-local, 224+.
+  # 100.64/10 is rejected too: NetBird allocates overlay peer addresses from
+  # it by default, and a bridge there shadows the overlay without any Docker
+  # network overlapping, so the conflict check below would not catch it.
+  case "$1" in
+    0.*|127.*|169.254.*|22[4-9].*|2[34][0-9].*|25[0-5].*) return 1 ;;
+    100.6[4-9].*|100.[7-9][0-9].*|100.1[01][0-9].*|100.12[0-7].*) return 1 ;;
+  esac
+  return 0
+}
+
+# Apply NETBIRD_DOCKER_SUBNET and derive the gateway (.1) and Traefik IP (.10)
+apply_docker_subnet_override() {
+  if [[ -n "${NETBIRD_DOCKER_SUBNET:-}" ]]; then
+    if ! valid_ipv4_slash24 "$NETBIRD_DOCKER_SUBNET"; then
+      echo "NETBIRD_DOCKER_SUBNET must be a unicast IPv4 /24 network like 10.123.45.0/24 (0/8, 127/8, 169.254/16, 100.64/10, and 224+ are not allowed), got: $NETBIRD_DOCKER_SUBNET" > /dev/stderr
+      exit 1
+    fi
+    DOCKER_SUBNET="$NETBIRD_DOCKER_SUBNET"
+  fi
+  local base="${DOCKER_SUBNET%.0/24}"
+  DOCKER_GATEWAY="${base}.1"
+  TRAEFIK_IP="${base}.10"
+  return 0
+}
+
+# check_docker_subnet_conflicts <compose network name>
+# Fail early if an existing Docker network overlaps DOCKER_SUBNET, instead
+# of letting "docker compose up" fail later. Host routes are not checked;
+# NETBIRD_DOCKER_SUBNET covers those cases.
+check_docker_subnet_conflicts() {
+  local expected_network="$1"
+  command -v docker &> /dev/null || return 0
+
+  # docker's own stderr is left visible on purpose: "is the daemon running"
+  # and socket permission errors are the actionable part. Only the exit status
+  # is handled here, because skipping the check silently would resurface later
+  # as a confusing "docker compose up" failure.
+  local ids_raw ls_status=0
+  ids_raw="$(docker network ls -q)" || ls_status=$?
+  if [[ "$ls_status" -ne 0 ]]; then
+    echo "ERROR: could not list the existing Docker networks (docker network ls exited $ls_status)." > /dev/stderr
+    echo "Without it this script cannot verify that $DOCKER_SUBNET is free." > /dev/stderr
+    echo "Make sure the Docker daemon is running and reachable by this user, then run this script again." > /dev/stderr
+    exit 1
+  fi
+
+  # Collect the IDs in an array so they reach docker as separate arguments
+  local network_ids=() id
+  while IFS= read -r id; do
+    if [[ -n "$id" ]]; then
+      network_ids+=("$id")
+    fi
+  done <<< "$ids_raw"
+
+  # No Docker networks at all: nothing can overlap, so there is nothing to check
+  [[ "${#network_ids[@]}" -gt 0 ]] || return 0
+
+  local inspect_output inspect_status=0
+  inspect_output="$(docker network inspect --format '{{.Name}}|{{range .IPAM.Config}}{{.Subnet}} {{end}}' "${network_ids[@]}")" || inspect_status=$?
+  if [[ "$inspect_status" -ne 0 ]]; then
+    echo "ERROR: could not inspect the existing Docker networks (docker network inspect exited $inspect_status)." > /dev/stderr
+    echo "Without it this script cannot verify that $DOCKER_SUBNET is free." > /dev/stderr
+    echo "If a Docker network was removed while this script was running, run the script again." > /dev/stderr
+    exit 1
+  fi
+
+  local name subnets subnet
+  while IFS='|' read -r name subnets; do
+    for subnet in $subnets; do
+      [[ "$subnet" =~ ^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+/[0-9]+$ ]] || continue
+      if [[ "$name" == "$expected_network" ]]; then
+        # Our own leftover network: compose reuses it as-is, so its subnet
+        # must match the one we render
+        if [[ "$subnet" != "$DOCKER_SUBNET" ]]; then
+          echo "ERROR: the Docker network '$name', left over from a previous NetBird install, uses $subnet instead of $DOCKER_SUBNET." > /dev/stderr
+          echo "docker compose would reuse it as-is, and the generated configuration would not match it." > /dev/stderr
+          echo "Remove it and run this script again:" > /dev/stderr
+          echo "  docker network rm $name" > /dev/stderr
+          exit 1
+        fi
+      elif cidrs_overlap "$DOCKER_SUBNET" "$subnet"; then
+        echo "ERROR: the existing Docker network '$name' ($subnet) overlaps $DOCKER_SUBNET, the subnet NetBird would use." > /dev/stderr
+        echo "That network is not managed by this script and is left untouched." > /dev/stderr
+        echo "Pick a free /24 for NetBird instead and run this script again:" > /dev/stderr
+        echo "  NETBIRD_DOCKER_SUBNET=10.123.45.0/24 ./getting-started-enterprise.sh" > /dev/stderr
+        exit 1
+      fi
+    done
+  done <<< "$inspect_output"
+  return 0
+}
+
 check_nb_domain() {
   local domain="$1"
   if [[ -z "$domain" ]]; then
@@ -224,6 +348,9 @@ wait_postgres() {
 init_environment() {
   check_openssl
   DOCKER_COMPOSE_COMMAND=$(check_docker_compose)
+  # Settle the subnet (and fail on conflicts) before the EULA and prompts
+  apply_docker_subnet_override
+  check_docker_subnet_conflicts "netbird"
 
   if [[ -f .env ]] || [[ -f docker-compose.yml ]] || [[ -f config.yaml ]]; then
     echo "Generated files already exist in $(pwd)."
@@ -273,6 +400,7 @@ init_environment() {
   echo "  Traffic flow: ${NETBIRD_TRAFFIC_FLOW}"
   echo "  Domain:       ${NETBIRD_DOMAIN}"
   echo "  ACME email:   ${NETBIRD_LETSENCRYPT_EMAIL}"
+  echo "  Subnet:       ${DOCKER_SUBNET} (Traefik at ${TRAEFIK_IP})"
   echo ""
   echo "Rendering files into $(pwd) ..."
   install -m 600 /dev/null .env
@@ -334,7 +462,12 @@ NETBIRD_DOMAIN=${NETBIRD_DOMAIN}
 # Reverse proxy (Traefik)
 NETBIRD_LETSENCRYPT_EMAIL=${NETBIRD_LETSENCRYPT_EMAIL}
 NETBIRD_TRAEFIK_TAG=${NETBIRD_TRAEFIK_TAG:-v3.6}
+# These three must stay in step with the /32 trust pins in config.yaml
+# (reverseProxy.trustedPeers/trustedHTTPProxies). Shell env vars override
+# this file at compose time.
 NETBIRD_TRAEFIK_IP=${TRAEFIK_IP}
+NETBIRD_NETWORK_SUBNET=${DOCKER_SUBNET}
+NETBIRD_NETWORK_GATEWAY=${DOCKER_GATEWAY}
 
 # Image tags. Default to "latest"
 NETBIRD_DASHBOARD_TAG=${NETBIRD_DASHBOARD_TAG:-latest}
@@ -417,6 +550,9 @@ render_compose_common() {
     networks:
       netbird:
         ipv4_address: ${NETBIRD_TRAEFIK_IP}
+        # Resolve the public domain inside this network (avoids hairpin NAT)
+        aliases:
+          - "${NETBIRD_DOMAIN}"
     command:
       # Logging
       - "--log.level=INFO"
@@ -660,8 +796,8 @@ networks:
     driver: bridge
     ipam:
       config:
-        - subnet: 172.30.0.0/24
-          gateway: 172.30.0.1
+        - subnet: ${NETBIRD_NETWORK_SUBNET}
+          gateway: ${NETBIRD_NETWORK_GATEWAY}
 EOF
 }
 

infrastructure_files/getting-started.sh

@@ -108,6 +108,20 @@ check_nb_domain() {
     echo "The NETBIRD_DOMAIN cannot be netbird.example.com" > /dev/stderr
     return 1
   fi
+
+  # Letters, digits, dots, and hyphens only; the domain is embedded in
+  # generated YAML and env files. This is not FQDN validation: "use-ip" and
+  # bare IP addresses are valid inputs here and both satisfy the pattern.
+  local re='^[A-Za-z0-9]([A-Za-z0-9.-]*[A-Za-z0-9])?$'
+  if [[ ! "$DOMAIN" =~ $re ]] || [[ "$DOMAIN" == *..* ]]; then
+    echo "The NETBIRD_DOMAIN may only contain letters, digits, dots, and hyphens, and cannot begin or end with a dot or hyphen." > /dev/stderr
+    return 1
+  fi
+
+  if [[ "${#DOMAIN}" -gt 253 ]]; then
+    echo "The NETBIRD_DOMAIN cannot be longer than 253 characters." > /dev/stderr
+    return 1
+  fi
   return 0
 }
 
@@ -337,6 +351,145 @@ wait_management_direct() {
   return 0
 }
 
+############################################
+# Docker Network Subnet Override and Conflict Check
+############################################
+
+ip_to_int() {
+  local a b c d
+  IFS=. read -r a b c d <<< "$1"
+  echo $(( (10#$a << 24) + (10#$b << 16) + (10#$c << 8) + 10#$d ))
+}
+
+# cidrs_overlap <cidr> <cidr> — succeeds if the networks overlap
+cidrs_overlap() {
+  local net1="${1%/*}" len1="${1#*/}" net2="${2%/*}" len2="${2#*/}"
+  local min_len=$(( len1 < len2 ? len1 : len2 ))
+  local mask=0
+  if [[ "$min_len" -gt 0 ]]; then
+    mask=$(( (0xFFFFFFFF << (32 - min_len)) & 0xFFFFFFFF ))
+  fi
+  [[ $(( $(ip_to_int "$net1") & mask )) -eq $(( $(ip_to_int "$net2") & mask )) ]]
+}
+
+# valid_ipv4_slash24 <cidr> — accepts a unicast IPv4 /24 like 10.123.45.0/24
+valid_ipv4_slash24() {
+  local octet='(25[0-5]|2[0-4][0-9]|1[0-9][0-9]|[1-9][0-9]|[0-9])'
+  local re="^${octet}\.${octet}\.${octet}\.0/24$"
+  [[ "$1" =~ $re ]] || return 1
+  # Reject non-unicast/reserved ranges: 0/8, loopback, link-local, 224+.
+  # 100.64/10 is rejected too: NetBird allocates overlay peer addresses from
+  # it by default, and a bridge there shadows the overlay without any Docker
+  # network overlapping, so the conflict check below would not catch it.
+  case "$1" in
+    0.*|127.*|169.254.*|22[4-9].*|2[34][0-9].*|25[0-5].*) return 1 ;;
+    100.6[4-9].*|100.[7-9][0-9].*|100.1[01][0-9].*|100.12[0-7].*) return 1 ;;
+  esac
+  return 0
+}
+
+# Apply NETBIRD_DOCKER_SUBNET and derive the gateway (.1) and Traefik IP (.10)
+apply_docker_subnet_override() {
+  if [[ -n "${NETBIRD_DOCKER_SUBNET:-}" ]]; then
+    if ! valid_ipv4_slash24 "$NETBIRD_DOCKER_SUBNET"; then
+      echo "NETBIRD_DOCKER_SUBNET must be a unicast IPv4 /24 network like 10.123.45.0/24 (0/8, 127/8, 169.254/16, 100.64/10, and 224+ are not allowed), got: $NETBIRD_DOCKER_SUBNET" > /dev/stderr
+      exit 1
+    fi
+    DOCKER_SUBNET="$NETBIRD_DOCKER_SUBNET"
+  fi
+  local base="${DOCKER_SUBNET%.0/24}"
+  DOCKER_GATEWAY="${base}.1"
+  TRAEFIK_IP="${base}.10"
+  return 0
+}
+
+# check_docker_subnet_conflicts <compose network name>
+# Fail early if an existing Docker network overlaps DOCKER_SUBNET, instead
+# of letting "docker compose up" fail later. Host routes are not checked;
+# NETBIRD_DOCKER_SUBNET covers those cases.
+check_docker_subnet_conflicts() {
+  local expected_network="$1"
+  command -v docker &> /dev/null || return 0
+
+  # docker's own stderr is left visible on purpose: "is the daemon running"
+  # and socket permission errors are the actionable part. Only the exit status
+  # is handled here, because skipping the check silently would resurface later
+  # as a confusing "docker compose up" failure.
+  local ids_raw ls_status=0
+  ids_raw="$(docker network ls -q)" || ls_status=$?
+  if [[ "$ls_status" -ne 0 ]]; then
+    echo "ERROR: could not list the existing Docker networks (docker network ls exited $ls_status)." > /dev/stderr
+    echo "Without it this script cannot verify that $DOCKER_SUBNET is free." > /dev/stderr
+    echo "Make sure the Docker daemon is running and reachable by this user, then run this script again." > /dev/stderr
+    exit 1
+  fi
+
+  # Collect the IDs in an array so they reach docker as separate arguments
+  local network_ids=() id
+  while IFS= read -r id; do
+    if [[ -n "$id" ]]; then
+      network_ids+=("$id")
+    fi
+  done <<< "$ids_raw"
+
+  # No Docker networks at all: nothing can overlap, so there is nothing to check
+  [[ "${#network_ids[@]}" -gt 0 ]] || return 0
+
+  local inspect_output inspect_status=0
+  inspect_output="$(docker network inspect --format '{{.Name}}|{{range .IPAM.Config}}{{.Subnet}} {{end}}' "${network_ids[@]}")" || inspect_status=$?
+  if [[ "$inspect_status" -ne 0 ]]; then
+    echo "ERROR: could not inspect the existing Docker networks (docker network inspect exited $inspect_status)." > /dev/stderr
+    echo "Without it this script cannot verify that $DOCKER_SUBNET is free." > /dev/stderr
+    echo "If a Docker network was removed while this script was running, run the script again." > /dev/stderr
+    exit 1
+  fi
+
+  local name subnets subnet
+  while IFS='|' read -r name subnets; do
+    for subnet in $subnets; do
+      [[ "$subnet" =~ ^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+/[0-9]+$ ]] || continue
+      if [[ "$name" == "$expected_network" ]]; then
+        # Our own leftover network: compose reuses it as-is, so its subnet
+        # must match the one we render
+        if [[ "$subnet" != "$DOCKER_SUBNET" ]]; then
+          echo "ERROR: the Docker network '$name', left over from a previous NetBird install, uses $subnet instead of $DOCKER_SUBNET." > /dev/stderr
+          echo "docker compose would reuse it as-is, and the generated configuration would not match it." > /dev/stderr
+          echo "Remove it and run this script again:" > /dev/stderr
+          echo "  docker network rm $name" > /dev/stderr
+          exit 1
+        fi
+      elif cidrs_overlap "$DOCKER_SUBNET" "$subnet"; then
+        echo "ERROR: the existing Docker network '$name' ($subnet) overlaps $DOCKER_SUBNET, the subnet NetBird would use." > /dev/stderr
+        echo "That network is not managed by this script and is left untouched." > /dev/stderr
+        echo "Pick a free /24 for NetBird instead and run this script again:" > /dev/stderr
+        echo "  NETBIRD_DOCKER_SUBNET=10.123.45.0/24 ./getting-started.sh" > /dev/stderr
+        exit 1
+      fi
+    done
+  done <<< "$inspect_output"
+  return 0
+}
+
+configure_docker_subnet() {
+  # Only the built-in Traefik mode pins a subnet; other modes let Docker pick
+  if [[ "$REVERSE_PROXY_TYPE" != "0" ]]; then
+    return 0
+  fi
+
+  # Skip our own network (<project>_netbird) in the conflict check. Compose
+  # derives the project name from the basename of the logical working directory
+  # (verified against Compose v5.4.0: a symlinked directory yields the symlink
+  # name, not its target), lowercases it, deletes every character outside
+  # [a-z0-9_-], then trims leading "_" and "-". Verified: "nb.test" -> "nbtest",
+  # "my nb" -> "mynb", "NetBird-1.0" -> "netbird-10". Networks are then named
+  # <project>_<key>.
+  local project
+  project="${COMPOSE_PROJECT_NAME:-$(basename "$PWD")}"
+  project=$(echo "$project" | tr '[:upper:]' '[:lower:]' | sed 's/[^a-z0-9_-]//g; s/^[_-]*//')
+  check_docker_subnet_conflicts "${project}_netbird"
+  return 0
+}
+
 ############################################
 # Initialization and Configuration
 ############################################
@@ -369,7 +522,11 @@ initialize_default_values() {
   BIND_LOCALHOST_ONLY="true"
   EXTERNAL_PROXY_NETWORK=""
 
-  # Traefik static IP within the internal bridge network
+  # Internal bridge network. Management and proxy trust forwarded headers
+  # from TRAEFIK_IP only, so all three values derive from the same /24.
+  # Override with NETBIRD_DOCKER_SUBNET.
+  DOCKER_SUBNET="172.30.0.0/24"
+  DOCKER_GATEWAY="172.30.0.1"
   TRAEFIK_IP="172.30.0.10"
 
   # NetBird Proxy configuration
@@ -665,8 +822,23 @@ init_environment() {
   check_docker_sock_perms
 
   initialize_default_values
+  apply_docker_subnet_override
+
+  # The agent-network preset pins built-in Traefik up front, so the subnet is
+  # already settled and a conflict can be reported before the prompts.
+  local subnet_checked="false"
+  if [[ "${NETBIRD_AGENT_NETWORK}" == "true" ]]; then
+    configure_docker_subnet
+    subnet_checked="true"
+  fi
+
   configure_domain
   configure_reverse_proxy
+  # Interactive runs only learn the proxy type above, and modes 1-5 never pin a
+  # subnet, so their check has to wait for that choice.
+  if [[ "$subnet_checked" != "true" ]]; then
+    configure_docker_subnet
+  fi
 
   check_jq
 
@@ -886,8 +1058,8 @@ networks:
     driver: bridge
     ipam:
       config:
-        - subnet: 172.30.0.0/24
-          gateway: 172.30.0.1
+        - subnet: $DOCKER_SUBNET
+          gateway: $DOCKER_GATEWAY
 EOF
   return 0
 }

infrastructure_files/migrate.sh

@@ -10,6 +10,9 @@
 #
 # Usage:
 #   ./migrate.sh [--install-dir /path/to/netbird] [--non-interactive]
+#
+# Environment:
+#   NETBIRD_DOCKER_SUBNET   /24 for the generated Docker network (default 172.30.0.0/24)
 
 set -euo pipefail
 
@@ -64,6 +67,14 @@ TRUSTED_PEERS=""
 MANAGEMENT_JSON_PATH=""
 BACKUP_DIR=""
 
+# Docker network for the generated Traefik compose. The Traefik container needs
+# a static address so the generated config can trust it, and Traefik's IP is
+# derived from the subnet, so both values stay in the same /24. Override with
+# NETBIRD_DOCKER_SUBNET.
+DOCKER_SUBNET="172.30.0.0/24"
+DOCKER_GATEWAY="172.30.0.1"
+TRAEFIK_IP="172.30.0.10"
+
 ############################################
 # Utility Functions
 ############################################
@@ -117,6 +128,159 @@ confirm_action() {
   return 0
 }
 
+############################################
+# Docker Network Subnet Override and Conflict Check
+############################################
+
+ip_to_int() {
+  local a b c d
+  IFS=. read -r a b c d <<< "$1"
+  echo $(( (10#$a << 24) + (10#$b << 16) + (10#$c << 8) + 10#$d ))
+}
+
+# cidrs_overlap <cidr> <cidr> — succeeds if the networks overlap
+cidrs_overlap() {
+  local net1="${1%/*}" len1="${1#*/}" net2="${2%/*}" len2="${2#*/}"
+  local min_len=$(( len1 < len2 ? len1 : len2 ))
+  local mask=0
+  if [[ "$min_len" -gt 0 ]]; then
+    mask=$(( (0xFFFFFFFF << (32 - min_len)) & 0xFFFFFFFF ))
+  fi
+  [[ $(( $(ip_to_int "$net1") & mask )) -eq $(( $(ip_to_int "$net2") & mask )) ]]
+}
+
+# valid_ipv4_slash24 <cidr> — accepts a unicast IPv4 /24 like 10.123.45.0/24
+valid_ipv4_slash24() {
+  local octet='(25[0-5]|2[0-4][0-9]|1[0-9][0-9]|[1-9][0-9]|[0-9])'
+  local re="^${octet}\.${octet}\.${octet}\.0/24$"
+  [[ "$1" =~ $re ]] || return 1
+  # Reject non-unicast/reserved ranges: 0/8, loopback, link-local, 224+.
+  # 100.64/10 is rejected too: NetBird allocates overlay peer addresses from
+  # it by default, and a bridge there shadows the overlay without any Docker
+  # network overlapping, so the conflict check below would not catch it.
+  case "$1" in
+    0.*|127.*|169.254.*|22[4-9].*|2[34][0-9].*|25[0-5].*) return 1 ;;
+    100.6[4-9].*|100.[7-9][0-9].*|100.1[01][0-9].*|100.12[0-7].*) return 1 ;;
+  esac
+  return 0
+}
+
+# Apply NETBIRD_DOCKER_SUBNET and derive the gateway (.1) and Traefik IP (.10).
+# Runs during preflight so a bad value fails before anything is touched.
+#
+# Unlike the getting-started scripts, the subnet has a single consumer here: the
+# generated docker-compose.yml. The reverseProxy trust pins in the generated
+# config.yaml are carried over verbatim from the old management.json (see
+# extract_config_values), so TRAEFIK_IP is deliberately not wired into them. If
+# an old config already pinned an address inside the default 172.30.0.0/24,
+# overriding the subnet will not update that pin.
+apply_docker_subnet_override() {
+  if [[ -n "${NETBIRD_DOCKER_SUBNET:-}" ]]; then
+    if ! valid_ipv4_slash24 "$NETBIRD_DOCKER_SUBNET"; then
+      log_error "NETBIRD_DOCKER_SUBNET must be a unicast IPv4 /24 network like 10.123.45.0/24 (0/8, 127/8, 169.254/16, 100.64/10, and 224+ are not allowed), got: $NETBIRD_DOCKER_SUBNET"
+      exit 1
+    fi
+    DOCKER_SUBNET="$NETBIRD_DOCKER_SUBNET"
+  fi
+  local base="${DOCKER_SUBNET%.0/24}"
+  DOCKER_GATEWAY="${base}.1"
+  TRAEFIK_IP="${base}.10"
+  return 0
+}
+
+# check_docker_subnet_conflicts <compose network name>
+# Fail before the new docker-compose.yml is written if an existing Docker
+# network overlaps DOCKER_SUBNET, instead of letting "docker compose up" fail
+# later. Host routes are not checked; NETBIRD_DOCKER_SUBNET covers those cases.
+check_docker_subnet_conflicts() {
+  local expected_network="$1"
+  command -v docker &> /dev/null || return 0
+
+  # docker's own stderr is left visible on purpose: "is the daemon running"
+  # and socket permission errors are the actionable part. Only the exit status
+  # is handled here, because skipping the check silently would resurface later
+  # as a confusing "docker compose up" failure.
+  local ids_raw ls_status=0
+  ids_raw="$(docker network ls -q)" || ls_status=$?
+  if [[ "$ls_status" -ne 0 ]]; then
+    log_error "Could not list the existing Docker networks (docker network ls exited $ls_status)."
+    echo "Without it this script cannot verify that $DOCKER_SUBNET is free."
+    echo "Make sure the Docker daemon is running and reachable by this user, then run this script again."
+    echo "The old deployment is stopped at this point; restart it with:"
+    echo "  bash $BACKUP_DIR/rollback.sh"
+    exit 1
+  fi
+
+  # Collect the IDs in an array so they reach docker as separate arguments
+  local network_ids=() id
+  while IFS= read -r id; do
+    if [[ -n "$id" ]]; then
+      network_ids+=("$id")
+    fi
+  done <<< "$ids_raw"
+
+  # No Docker networks at all: nothing can overlap, so there is nothing to check
+  [[ "${#network_ids[@]}" -gt 0 ]] || return 0
+
+  local inspect_output inspect_status=0
+  inspect_output="$(docker network inspect --format '{{.Name}}|{{range .IPAM.Config}}{{.Subnet}} {{end}}' "${network_ids[@]}")" || inspect_status=$?
+  if [[ "$inspect_status" -ne 0 ]]; then
+    log_error "Could not inspect the existing Docker networks (docker network inspect exited $inspect_status)."
+    echo "Without it this script cannot verify that $DOCKER_SUBNET is free."
+    echo "If a Docker network was removed while this script was running, run the script again."
+    echo "The old deployment is stopped at this point; restart it with:"
+    echo "  bash $BACKUP_DIR/rollback.sh"
+    exit 1
+  fi
+
+  local name subnets subnet
+  while IFS='|' read -r name subnets; do
+    for subnet in $subnets; do
+      [[ "$subnet" =~ ^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+/[0-9]+$ ]] || continue
+      if [[ "$name" == "$expected_network" ]]; then
+        # Our own leftover network: compose reuses it as-is, so its subnet
+        # must match the one we render
+        if [[ "$subnet" != "$DOCKER_SUBNET" ]]; then
+          log_error "The Docker network '$name', left over from an earlier run, uses $subnet instead of $DOCKER_SUBNET."
+          echo "docker compose would reuse it as-is, and the generated configuration would not match it."
+          echo "Remove it and run this script again:"
+          echo "  docker network rm $name"
+          exit 1
+        fi
+      elif cidrs_overlap "$DOCKER_SUBNET" "$subnet"; then
+        log_error "The existing Docker network '$name' ($subnet) overlaps $DOCKER_SUBNET, the subnet NetBird would use."
+        echo "That network is not managed by this script and is left untouched."
+        echo "If it belongs to the old NetBird deployment and is no longer in use, remove it:"
+        echo "  docker network rm $name"
+        echo "Otherwise pick a free /24 for NetBird instead and run this script again:"
+        echo "  NETBIRD_DOCKER_SUBNET=10.123.45.0/24 ./migrate.sh"
+        exit 1
+      fi
+    done
+  done <<< "$inspect_output"
+  return 0
+}
+
+# Only reached on the automatic (embedded Caddy) path, which is the only one
+# that generates a compose file pinning a subnet; the exposed-ports compose for
+# custom proxies lets Docker pick.
+configure_docker_subnet() {
+  # Skip our own network (<project>_netbird) in the conflict check. start_new_services
+  # runs "cd $INSTALL_DIR && compose up", a logical cd, and compose derives the
+  # project name from the basename of that logical path -- so resolve it the same
+  # way with a plain "pwd" (a relative --install-dir still yields an absolute
+  # path, and a symlinked install dir keeps the symlink name, which is what
+  # compose sees). "pwd -P" here would resolve the symlink target and no longer
+  # match. Compose then lowercases, deletes every character outside [a-z0-9_-],
+  # and trims leading "_" and "-"; verified against Compose v5.4.0 that
+  # "nb.test" -> "nbtest" and "my nb" -> "mynb".
+  local project
+  project="${COMPOSE_PROJECT_NAME:-$(basename "$(cd -- "$INSTALL_DIR" && pwd)")}"
+  project=$(echo "$project" | tr '[:upper:]' '[:lower:]' | sed 's/[^a-z0-9_-]//g; s/^[_-]*//')
+  check_docker_subnet_conflicts "${project}_netbird"
+  return 0
+}
+
 ############################################
 # Phase 0: Preflight & Detection
 ############################################
@@ -577,6 +741,7 @@ print_detection_summary() {
     echo "  Migration mode:     AUTOMATIC"
     echo "  A Traefik-based docker-compose.yml will be generated and services"
     echo "  will be stopped and restarted automatically."
+    echo "  Docker subnet:      $DOCKER_SUBNET (Traefik at $TRAEFIK_IP)"
   else
     echo "  Migration mode:     MANUAL"
     echo "  New config files will be generated. You will need to stop old"
@@ -843,7 +1008,7 @@ services:
     restart: unless-stopped
     networks:
       netbird:
-        ipv4_address: 172.30.0.10
+        ipv4_address: ${TRAEFIK_IP}
     command:
       # Logging
       - "--log.level=INFO"
@@ -952,8 +1117,8 @@ networks:
     driver: bridge
     ipam:
       config:
-        - subnet: 172.30.0.0/24
-          gateway: 172.30.0.1
+        - subnet: ${DOCKER_SUBNET}
+          gateway: ${DOCKER_GATEWAY}
 EOF
 
   log_success "Generated docker-compose.yml"
@@ -1226,6 +1391,10 @@ main() {
         echo "  --install-dir DIR    Path to existing NetBird installation"
         echo "  --non-interactive    Skip confirmation prompts (for automation)"
         echo "  -h, --help           Show this help message"
+        echo ""
+        echo "Environment:"
+        echo "  NETBIRD_DOCKER_SUBNET  /24 for the generated Docker network"
+        echo "                         (default $DOCKER_SUBNET; Traefik takes .10)"
         exit 0
         ;;
       *)
@@ -1240,6 +1409,7 @@ main() {
 
   # Phase 0: Preflight & Detection
   check_dependencies
+  apply_docker_subnet_override
   detect_install_dir
   validate_old_setup
   check_already_migrated
@@ -1261,6 +1431,10 @@ main() {
     # Stop old containers BEFORE overwriting docker-compose.yml
     stop_old_services
 
+    # "compose down" above released the old deployment's networks, so anything
+    # still overlapping now is a network this script must not touch
+    configure_docker_subnet
+
     # Phase 2 + 3: Generate new configuration files
     generate_config_yaml
     generate_dashboard_env

util/common.go

@@ -3,69 +3,18 @@ package util
 import (
 	"os"
 	"os/exec"
-	"runtime"
-	"slices"
 
 	"github.com/skratchdot/open-golang/open"
 )
 
-const (
-	// envBrowser overrides the browser OpenBrowser launches
-	envBrowser = "BROWSER"
-	// envDesktopSession and envXDGCurrentDesktop are what xdg-open uses to pick a handler
-	envDesktopSession    = "DESKTOP_SESSION"
-	envXDGCurrentDesktop = "XDG_CURRENT_DESKTOP"
-	// envDisplay and envWaylandDisplay are what a graphical browser needs to reach a display
-	envDisplay        = "DISPLAY"
-	envWaylandDisplay = "WAYLAND_DISPLAY"
-	// envXDGSessionType names the session kind, e.g. tty, x11 or wayland
-	envXDGSessionType = "XDG_SESSION_TYPE"
-)
-
 // OpenBrowser opens the URL in a browser, respecting the BROWSER environment variable.
 func OpenBrowser(url string) error {
-	if browser := os.Getenv(envBrowser); browser != "" {
+	if browser := os.Getenv("BROWSER"); browser != "" {
 		return exec.Command(browser, url).Start()
 	}
 	return open.Run(url)
 }
 
-// browserSessionEnvVars returns the variables that decide whether OpenBrowser can open a URL.
-// DISPLAY and WAYLAND_DISPLAY are exactly what xdg-open's own has_display() checks, and without
-// them it degrades to terminal browsers. BROWSER is the explicit override both xdg-open and
-// OpenBrowser honor first. DESKTOP_SESSION and XDG_CURRENT_DESKTOP only tell xdg-open which
-// desktop-specific opener to prefer, so they are weaker evidence, kept because the previous
-// detection relied on them alone and dropping them would demote sessions that work today.
-func browserSessionEnvVars() []string {
-	return []string{envDisplay, envWaylandDisplay, envBrowser, envDesktopSession, envXDGCurrentDesktop}
-}
-
-// graphicalXDGSessionTypes are the systemd-logind session types that come with a display. The
-// other documented values are "tty" and "unspecified"; anything unrecognized is treated as no
-// display, so an unknown value picks the device code flow, which works without a browser.
-func graphicalXDGSessionTypes() []string {
-	return []string{"x11", "wayland", "mir"}
-}
-
-// HasGraphicalSession reports whether this process can open a browser and serve a loopback
-// redirect back to it. Windows and macOS always can. On Linux and FreeBSD the answer is env
-// based, so it only holds for a process started from the graphical session itself: a service
-// does not inherit those variables and always reports false, which is why callers running in
-// the user's session pass their own answer to the daemon.
-func HasGraphicalSession() bool {
-	if runtime.GOOS != "linux" && runtime.GOOS != "freebsd" {
-		return true
-	}
-
-	for _, env := range browserSessionEnvVars() {
-		if os.Getenv(env) != "" {
-			return true
-		}
-	}
-
-	return slices.Contains(graphicalXDGSessionTypes(), os.Getenv(envXDGSessionType))
-}
-
 // SliceDiff returns the elements in slice `x` that are not in slice `y`
 func SliceDiff(x, y []string) []string {
 	mapY := make(map[string]struct{}, len(y))

util/session_test.go

@@ -1,50 +0,0 @@
-package util
-
-import (
-	"os"
-	"runtime"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-)
-
-func TestHasGraphicalSession(t *testing.T) {
-	if runtime.GOOS != "linux" && runtime.GOOS != "freebsd" {
-		assert.True(t, HasGraphicalSession(), "%s always has a graphical session", runtime.GOOS)
-		return
-	}
-
-	// clear anything inherited from the session running the test, restored on cleanup
-	for _, env := range append(browserSessionEnvVars(), envXDGSessionType) {
-		t.Setenv(env, "")
-		os.Unsetenv(env)
-	}
-
-	assert.False(t, HasGraphicalSession(), "no session variables means no graphical session")
-
-	tests := []struct {
-		env      string
-		value    string
-		expected bool
-	}{
-		{env: envDisplay, value: ":0", expected: true},
-		{env: envWaylandDisplay, value: "wayland-0", expected: true},
-		{env: envDesktopSession, value: "gnome", expected: true},
-		{env: envXDGCurrentDesktop, value: "KDE", expected: true},
-		{env: envBrowser, value: "firefox", expected: true},
-		{env: envXDGSessionType, value: "wayland", expected: true},
-		{env: envXDGSessionType, value: "x11", expected: true},
-		{env: envXDGSessionType, value: "mir", expected: true},
-		{env: envXDGSessionType, value: "tty", expected: false},
-		{env: envXDGSessionType, value: "unspecified", expected: false},
-		// an unrecognized type must not be read as a display: the device code flow works anyway
-		{env: envXDGSessionType, value: "something-new", expected: false},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.env+"="+tt.value, func(t *testing.T) {
-			t.Setenv(tt.env, tt.value)
-			assert.Equal(t, tt.expected, HasGraphicalSession(), "%s=%s", tt.env, tt.value)
-		})
-	}
-}