Compare commits

...

3 Commits

Author SHA1 Message Date
Zoltan Papp
16544dbc58 [client] Pass stored email as login hint from UI and keep it on logout (#7199)
* [client] Pass stored email as login hint from UI and keep it on logout

Follow the CLI pattern: the Wails UI now reads the account email from the
user-owned profile state file and passes it as the OIDC login_hint on login
and session extend, since the daemon-side fallback runs as root and cannot
see the user's state file. Logout no longer deletes the stored email, so a
later login preselects the account at the IdP; profile removal remains the
operation that deletes it.

* [client] Log ignored profile lookup errors in extend-session hint fallback
2026-08-15 11:21:57 +02:00
Zoltan Papp
f458c1f265 [client] Skip IPv6 route tests when the default nexthop is unusable (#7212)
* [client] Skip IPv6 route tests when the default nexthop is unusable

ensureIPv6DefaultRoute treated a successful netlink RouteAdd as proof that
a usable IPv6 nexthop exists. Installing ::/0 via loopback can succeed while
the kernel still rejects that nexthop for a concrete prefix, which surfaced
on ubuntu22/20260810.260 runners as:

    add route to table: netlink add route: invalid argument

Probe the resolved nexthop by installing and removing a discard-prefix route
through the same code path the tests use, and skip when it fails. EEXIST
means the nexthop already carries a route, so it counts as usable.

* [client] Probe the IPv6 nexthop through raw netlink

addRoute swallows EAFNOSUPPORT and EOPNOTSUPP via isOpErr, so a nil return
did not prove the probe route was installed. Call netlink directly so an
unsupported operation skips the test instead of passing as usable.
2026-08-15 10:13:06 +02:00
Viktor Liu
ec6f1b8c27 [client] Rank Windows route candidates by combined route and interface metric (#7210) 2026-08-15 09:06:22 +02:00
7 changed files with 169 additions and 28 deletions

View File

@@ -87,9 +87,10 @@ func (pm *ProfileManager) SetActiveProfileState(state *ProfileState) error {
// RemoveProfileState deletes the per-profile state file (which holds the
// account email used for the SSO login hint and the UI display). Called after
// a successful logout so a logged-out profile no longer shows a stale account
// email. The state file only stores the email, so deleting it is equivalent to
// clearing it; the next SSO login recreates it. A missing file is not an error.
// profile removal; logout keeps the file so the next login can pass the email
// as the login_hint. The state file only stores the email, so deleting it is
// equivalent to clearing it; the next SSO login recreates it. A missing file
// is not an error.
func (pm *ProfileManager) RemoveProfileState(profileName string) error {
configDir, err := getConfigDir()
if err != nil {

View File

@@ -0,0 +1,82 @@
//go:build windows
package systemops
import (
"math"
"testing"
"github.com/stretchr/testify/assert"
)
func TestSortRouteCandidates(t *testing.T) {
tests := []struct {
name string
candidates []candidateRoute
wantOrder []uint32
}{
{
name: "longest prefix wins over metrics",
candidates: []candidateRoute{
{interfaceIndex: 1, prefixLength: 0, routeMetric: 0, interfaceMetric: 5},
{interfaceIndex: 2, prefixLength: 24, routeMetric: 100, interfaceMetric: 50},
},
wantOrder: []uint32{2, 1},
},
{
// Windows ranks equal-length prefixes by route metric + interface metric,
// so a higher route metric on a low metric interface can still win.
name: "combined metric beats route metric alone",
candidates: []candidateRoute{
{interfaceIndex: 8, prefixLength: 0, routeMetric: 0, interfaceMetric: 100},
{interfaceIndex: 5, prefixLength: 0, routeMetric: 10, interfaceMetric: 5},
},
wantOrder: []uint32{5, 8},
},
{
name: "lower combined metric wins",
candidates: []candidateRoute{
{interfaceIndex: 5, prefixLength: 0, routeMetric: 300, interfaceMetric: 5},
{interfaceIndex: 8, prefixLength: 0, routeMetric: 0, interfaceMetric: 100},
},
wantOrder: []uint32{8, 5},
},
{
name: "equal combined metric falls back to route metric",
candidates: []candidateRoute{
{interfaceIndex: 1, prefixLength: 0, routeMetric: 20, interfaceMetric: 10},
{interfaceIndex: 2, prefixLength: 0, routeMetric: 5, interfaceMetric: 25},
},
wantOrder: []uint32{2, 1},
},
{
// The metrics are uint32 on the Windows side, so the sum must not wrap.
name: "combined metric beyond the uint32 range",
candidates: []candidateRoute{
{interfaceIndex: 1, prefixLength: 0, routeMetric: math.MaxUint32, interfaceMetric: 5},
{interfaceIndex: 2, prefixLength: 0, routeMetric: math.MaxUint32 - 10, interfaceMetric: 5},
},
wantOrder: []uint32{2, 1},
},
{
name: "unknown interface metric ranks on route metric only",
candidates: []candidateRoute{
{interfaceIndex: 1, prefixLength: 0, routeMetric: 30, interfaceMetric: -1},
{interfaceIndex: 2, prefixLength: 0, routeMetric: 5, interfaceMetric: 10},
},
wantOrder: []uint32{2, 1},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
sortRouteCandidates(tt.candidates)
got := make([]uint32, 0, len(tt.candidates))
for _, c := range tt.candidates {
got = append(got, c.interfaceIndex)
}
assert.Equal(t, tt.wantOrder, got)
})
}
}

View File

@@ -882,26 +882,40 @@ func getInterfaceMetric(interfaceIndex uint32, family int16) int {
return int(ipInterfaceRow.Metric)
}
// sortRouteCandidates sorts route candidates by priority: prefix length -> route metric -> interface metric
// sortRouteCandidates sorts route candidates by priority: prefix length -> combined metric -> route metric.
// Windows prefers the longest matching prefix and, among prefixes of the same length, the lowest metric, see
// https://learn.microsoft.com/en-us/windows-hardware/customize/desktop/unattend/microsoft-windows-tcpip-interfaces-interface-routes-route-metric
func sortRouteCandidates(candidates []candidateRoute) {
sort.Slice(candidates, func(i, j int) bool {
if candidates[i].prefixLength != candidates[j].prefixLength {
return candidates[i].prefixLength > candidates[j].prefixLength
}
if candidates[i].routeMetric != candidates[j].routeMetric {
return candidates[i].routeMetric < candidates[j].routeMetric
mi, mj := combinedMetric(candidates[i]), combinedMetric(candidates[j])
if mi != mj {
return mi < mj
}
return candidates[i].interfaceMetric < candidates[j].interfaceMetric
return candidates[i].routeMetric < candidates[j].routeMetric
})
}
// combinedMetric returns the effective metric Windows uses to rank routes with an equal prefix length:
// the sum of the route metric and the metric of the interface the route is on, see
// https://learn.microsoft.com/en-us/windows-server/networking/technologies/network-subsystem/net-sub-interface-metric
// An unknown interface metric contributes nothing.
func combinedMetric(candidate candidateRoute) uint64 {
if candidate.interfaceMetric < 0 {
return uint64(candidate.routeMetric)
}
return uint64(candidate.routeMetric) + uint64(candidate.interfaceMetric)
}
// GetBestInterface finds the best interface for reaching a destination,
// excluding the VPN interface to avoid routing loops.
//
// Route selection priority:
// 1. Longest prefix match (most specific route)
// 2. Lowest route metric
// 3. Lowest interface metric
// 2. Lowest combined metric (route metric + interface metric)
// 3. Lowest route metric.
func GetBestInterface(dest netip.Addr, vpnIntf string) (*net.Interface, error) {
var skipInterfaceIndex int
if vpnIntf != "" {
@@ -925,7 +939,6 @@ func GetBestInterface(dest netip.Addr, vpnIntf string) (*net.Interface, error) {
return nil, fmt.Errorf("no route to %s", dest)
}
// Sort routes: prefix length -> route metric -> interface metric
sortRouteCandidates(candidates)
for _, candidate := range candidates {

View File

@@ -5,6 +5,7 @@ package systemops
import (
"errors"
"net"
"net/netip"
"syscall"
"testing"
@@ -29,6 +30,7 @@ func ensureIPv6DefaultRoute(t *testing.T) {
}
if err := netlink.RouteAdd(route); err != nil {
if errors.Is(err, syscall.EEXIST) {
requireUsableIPv6Nexthop(t)
return
}
t.Skipf("install IPv6 fallback default route: %v", err)
@@ -38,4 +40,36 @@ func ensureIPv6DefaultRoute(t *testing.T) {
t.Logf("delete IPv6 fallback default route: %v", err)
}
})
requireUsableIPv6Nexthop(t)
}
// requireUsableIPv6Nexthop skips the test unless the resolved IPv6 default
// nexthop can actually carry a route. Installing the default route succeeding
// does not imply the kernel accepts it as a nexthop for a concrete prefix.
func requireUsableIPv6Nexthop(t *testing.T) {
t.Helper()
nexthop, err := GetNextHop(netip.IPv6Unspecified())
if err != nil {
t.Skipf("resolve IPv6 default nexthop: %v", err)
}
probe := &netlink.Route{
Scope: netlink.SCOPE_UNIVERSE,
Table: syscall.RT_TABLE_MAIN,
Family: netlink.FAMILY_V6,
Dst: &net.IPNet{IP: net.ParseIP("100::64"), Mask: net.CIDRMask(128, 128)},
}
require.NoError(t, addNextHop(nexthop, probe), "build IPv6 probe route")
switch err := netlink.RouteAdd(probe); {
case err == nil:
if err := netlink.RouteDel(probe); err != nil && !errors.Is(err, syscall.ESRCH) {
t.Logf("delete IPv6 probe route: %v", err)
}
case errors.Is(err, syscall.EEXIST):
default:
t.Skipf("IPv6 nexthop %s unusable for route installation: %v", nexthop, err)
}
}

View File

@@ -6,9 +6,11 @@ import (
"context"
"time"
log "github.com/sirupsen/logrus"
"google.golang.org/grpc/codes"
gstatus "google.golang.org/grpc/status"
"github.com/netbirdio/netbird/client/internal/profilemanager"
"github.com/netbirdio/netbird/client/proto"
)
@@ -60,9 +62,19 @@ func (s *Session) RequestExtend(ctx context.Context, p ExtendStartParams) (Exten
// a request from the UI implies a graphical session, which the daemon cannot detect itself
req := &proto.RequestExtendAuthSessionRequest{HasGraphicalSession: true}
if p.Hint != "" {
h := p.Hint
req.Hint = &h
hint := p.Hint
if hint == "" {
pm := profilemanager.NewProfileManager()
if active, perr := pm.GetActiveProfile(); perr != nil {
log.Debugf("failed to get active profile for login hint: %v", perr)
} else if state, serr := pm.GetProfileState(active.ID); serr != nil {
log.Debugf("failed to get profile state for login hint: %v", serr)
} else {
hint = state.Email
}
}
if hint != "" {
req.Hint = &hint
}
resp, err := cli.RequestExtendAuthSession(ctx, req)

View File

@@ -123,8 +123,16 @@ func (s *Connection) Login(ctx context.Context, p LoginParams) (LoginResult, err
if p.PreSharedKey != "" {
req.OptionalPreSharedKey = ptrStr(p.PreSharedKey)
}
if p.Hint != "" {
req.Hint = ptrStr(p.Hint)
hint := p.Hint
if hint == "" && profileID != "" {
if state, serr := profilemanager.NewProfileManager().GetProfileState(profilemanager.ID(profileID)); serr == nil {
hint = state.Email
} else {
log.Debugf("failed to get profile state for login hint: %v", serr)
}
}
if hint != "" {
req.Hint = ptrStr(hint)
}
resp, err := cli.Login(ctx, req)
@@ -228,16 +236,6 @@ func (s *Connection) Logout(ctx context.Context, p LogoutParams) error {
return s.classifyDaemonError(err)
}
// The daemon runs as root and can't reach the user-owned per-profile state
// file holding the account email (see Profiles.List), so clear the stale
// email here; the next SSO login recreates it.
if p.ProfileName != "" {
if err := profilemanager.NewProfileManager().RemoveProfileState(p.ProfileName); err != nil {
// Non-fatal: the logout itself succeeded.
log.Warnf("failed to remove profile state for %s: %v", p.ProfileName, err)
}
}
return nil
}
@@ -261,7 +259,7 @@ func (s *Connection) waitSSOLogin(ctx context.Context, p WaitSSOParams) (string,
// Persist the account email the same way the CLI does after its own
// WaitSSOLogin: the daemon returns it but cannot store it, since it runs as
// root and the per-profile state file is user-owned (see Logout below).
// root and the per-profile state file is user-owned (see Profiles.List).
// Without this the profile has no email, so Profiles.List shows no account
// and later logins and session extends go out without a login_hint —
// leaving the IdP to guess which account was meant.

View File

@@ -162,8 +162,9 @@ func (s *Profiles) Remove(ctx context.Context, p ProfileRef) error {
}
// The daemon deletes what it owns but runs as root, so it leaves the
// user-owned state file holding the account email behind (same split as
// Connection.Logout). Legacy profiles are keyed by name rather than by a
// user-owned state file holding the account email behind. Logout keeps the
// email on purpose so later logins can pass it as the login_hint; profile
// removal is what deletes it. Legacy profiles are keyed by name rather than by a
// generated ID, so a recreated profile of the same name would inherit the
// deleted one's email and offer it as the login_hint.
//