Merge remote-tracking branch 'origin/main' into embedded-vnc

# Conflicts:
#	client/cmd/up.go
#	client/internal/engine.go
#	client/internal/profilemanager/config.go
#	client/proto/daemon.pb.go
#	client/server/mdm.go
#	client/server/server.go
#	client/status/status.go
#	client/ui/main.go
#	client/ui/services/windowmanager.go
#	management/internals/shared/grpc/conversion.go
#	management/server/store/sql_store.go
#	management/server/store/sql_store_test.go
#	shared/management/networkmap/envelope.go
#	shared/management/proto/management.pb.go
This commit is contained in:
Viktor Liu
2026-10-19 19:49:54 +09:00
804 changed files with 72029 additions and 25943 deletions
-54
View File
@@ -1,54 +0,0 @@
package server
import (
"context"
firewall "github.com/netbirdio/netbird/client/firewall/manager"
"github.com/netbirdio/netbird/client/proto"
)
func (s *Server) ForwardingRules(context.Context, *proto.EmptyRequest) (*proto.ForwardingRulesResponse, error) {
s.mutex.Lock()
defer s.mutex.Unlock()
rules := s.statusRecorder.ForwardingRules()
responseRules := make([]*proto.ForwardingRule, 0, len(rules))
for _, rule := range rules {
respRule := &proto.ForwardingRule{
Protocol: string(rule.Protocol),
DestinationPort: portToProto(rule.DestinationPort),
TranslatedAddress: rule.TranslatedAddress.String(),
TranslatedHostname: s.hostNameByTranslateAddress(rule.TranslatedAddress.String()),
TranslatedPort: portToProto(rule.TranslatedPort),
}
responseRules = append(responseRules, respRule)
}
return &proto.ForwardingRulesResponse{Rules: responseRules}, nil
}
func (s *Server) hostNameByTranslateAddress(ip string) string {
hostName, ok := s.statusRecorder.PeerByIP(ip)
if !ok {
return ip
}
return hostName
}
func portToProto(port firewall.Port) *proto.PortInfo {
var portInfo proto.PortInfo
if !port.IsRange {
portInfo.PortSelection = &proto.PortInfo_Port{Port: uint32(port.Values[0])}
} else {
portInfo.PortSelection = &proto.PortInfo_Range_{
Range: &proto.PortInfo_Range{
Start: uint32(port.Values[0]),
End: uint32(port.Values[1]),
},
}
}
return &portInfo
}
+332
View File
@@ -0,0 +1,332 @@
package server
import (
"context"
"errors"
"path/filepath"
"testing"
"time"
"github.com/stretchr/testify/require"
"github.com/netbirdio/netbird/client/internal"
"github.com/netbirdio/netbird/client/internal/auth"
"github.com/netbirdio/netbird/client/internal/ipcauth"
"github.com/netbirdio/netbird/client/internal/profilemanager"
"github.com/netbirdio/netbird/client/mdm"
"github.com/netbirdio/netbird/client/proto"
)
type jwtStoringMDMFetcher struct {
cache *jwtCache
caller ipcauth.Identity
generation uint64
}
func (f *jwtStoringMDMFetcher) Fetch() map[string]any {
f.generation = f.cache.currentGeneration()
f.cache.store("previous-profile-token", f.caller, time.Minute, f.generation)
return nil
}
type stubOAuthFlow struct {
token auth.TokenInfo
onWait func()
}
func (f *stubOAuthFlow) RequestAuthInfo(context.Context) (auth.AuthFlowInfo, error) {
return auth.AuthFlowInfo{}, nil
}
func (f *stubOAuthFlow) WaitToken(context.Context, auth.AuthFlowInfo) (auth.TokenInfo, error) {
if f.onWait != nil {
f.onWait()
}
return f.token, nil
}
func (f *stubOAuthFlow) GetClientID(context.Context) string {
return "stub-client"
}
func TestWaitSSOLogin_WrongAccountArmsPromptAndFails(t *testing.T) {
s := newSSOTestServer(t, "user@example.com", false, "other@example.com")
attempts := 0
s.loginAttemptFn = func(context.Context, string, string) (internal.StatusType, error) {
attempts++
return "", nil
}
resp, err := s.WaitSSOLogin(callerCtx(t), &proto.WaitSSOLoginRequest{UserCode: "code"})
require.Error(t, err)
require.Nil(t, resp)
require.Equal(t, 0, attempts, "the wrong account's token reached the management login")
require.True(t, s.forceAccountPrompt, "the next login was not armed to ask for the account")
require.Nil(t, s.oauthAuthFlow.flow, "the mismatched flow stayed cached for reuse")
status, stateErr := internal.CtxGetState(s.rootCtx).Status()
require.NoError(t, stateErr)
require.Equal(t, internal.StatusNeedsLogin, status, "the mismatch must stay retryable")
}
func TestWaitSSOLogin_WrongAccountAfterPromptProceeds(t *testing.T) {
s := newSSOTestServer(t, "user@example.com", true, "other@example.com")
attempts := 0
s.loginAttemptFn = func(context.Context, string, string) (internal.StatusType, error) {
attempts++
return "", nil
}
resp, err := s.WaitSSOLogin(callerCtx(t), &proto.WaitSSOLoginRequest{UserCode: "code"})
require.NoError(t, err, "a prompted round must not error again on a mismatch")
require.NotNil(t, resp)
require.Equal(t, "other@example.com", resp.Email)
require.Equal(t, 1, attempts)
require.False(t, s.forceAccountPrompt)
}
func TestWaitSSOLogin_MatchingAccountProceeds(t *testing.T) {
s := newSSOTestServer(t, "user@example.com", false, "User@Example.com")
attempts := 0
s.loginAttemptFn = func(context.Context, string, string) (internal.StatusType, error) {
attempts++
return "", nil
}
resp, err := s.WaitSSOLogin(callerCtx(t), &proto.WaitSSOLoginRequest{UserCode: "code"})
require.NoError(t, err)
require.NotNil(t, resp)
require.Equal(t, 1, attempts)
require.False(t, s.forceAccountPrompt)
}
func TestWaitSSOLogin_NoHintIsNotJudged(t *testing.T) {
s := newSSOTestServer(t, "", false, "whoever@example.com")
attempts := 0
s.loginAttemptFn = func(context.Context, string, string) (internal.StatusType, error) {
attempts++
return "", nil
}
_, err := s.WaitSSOLogin(callerCtx(t), &proto.WaitSSOLoginRequest{UserCode: "code"})
require.NoError(t, err)
require.Equal(t, 1, attempts)
require.False(t, s.forceAccountPrompt)
}
func TestSwitchProfile_DropsAccountPromptAndPendingFlow(t *testing.T) {
s, ctx, _, _, _ := setupServerWithProfile(t)
s.forceAccountPrompt = true
cancelled := false
s.oauthAuthFlow = oauthAuthFlow{
flow: &stubOAuthFlow{},
hint: "user@example.com",
waitCancel: func() { cancelled = true },
}
extendCancelled := false
s.extendAuthSessionFlow.Set(&stubOAuthFlow{}, auth.AuthFlowInfo{DeviceCode: "device"})
s.extendAuthSessionFlow.SetWaitCancel(func() { extendCancelled = true })
_, err := s.SwitchProfile(ctx, nil)
require.NoError(t, err)
require.False(t, s.forceAccountPrompt, "the prompt flag leaked across a profile switch")
require.Nil(t, s.oauthAuthFlow.flow, "the previous profile's flow leaked across a profile switch")
require.Empty(t, s.oauthAuthFlow.hint)
require.True(t, cancelled, "the pending wait was not cancelled")
require.True(t, extendCancelled, "the pending extend wait was not cancelled")
_, _, pending := s.extendAuthSessionFlow.Get()
require.False(t, pending, "the previous profile's extend flow leaked across a profile switch")
}
func TestLogin_ProfileSwitchDropsAccountPromptAndPendingFlow(t *testing.T) {
s, _, _, username, cfgPath := setupServerWithProfile(t)
s.rootCtx = internal.CtxInitState(context.Background())
s.isLoginRequiredFn = func(context.Context) (bool, error) {
return true, nil
}
other := "other-profile"
otherPath := filepath.Join(filepath.Dir(cfgPath), other+".json")
_, err := profilemanager.UpdateOrCreateConfig(profilemanager.ConfigInput{
ConfigPath: otherPath,
ManagementURL: "https://api.netbird.io:443",
})
require.NoError(t, err)
breakProfilePrivateKey(t, otherPath)
s.forceAccountPrompt = true
cancelled := false
s.oauthAuthFlow = oauthAuthFlow{
flow: &stubOAuthFlow{},
hint: "user@example.com",
waitCancel: func() { cancelled = true },
}
extendCancelled := false
s.extendAuthSessionFlow.Set(&stubOAuthFlow{}, auth.AuthFlowInfo{DeviceCode: "device"})
s.extendAuthSessionFlow.SetWaitCancel(func() { extendCancelled = true })
generation := s.jwtCache.currentGeneration()
_, err = s.Login(userCtx(), &proto.LoginRequest{ProfileName: &other, Username: &username})
require.Error(t, err, "the broken key must stop the login before a flow is built")
active, err := s.profileManager.GetActiveProfileState()
require.NoError(t, err)
require.Equal(t, profilemanager.ID(other), active.ID, "the login did not switch the profile")
require.False(t, s.forceAccountPrompt, "the prompt flag leaked across a login-driven profile switch")
require.Nil(t, s.oauthAuthFlow.flow, "the previous profile's flow leaked across a login-driven profile switch")
require.Empty(t, s.oauthAuthFlow.hint)
require.True(t, cancelled, "the pending wait was not cancelled")
require.True(t, extendCancelled, "the pending extend wait was not cancelled")
_, _, pending := s.extendAuthSessionFlow.Get()
require.False(t, pending, "the previous profile's extend flow leaked across a login-driven profile switch")
require.Greater(t, s.jwtCache.currentGeneration(), generation, "the previous profile's JWT cache survived a login-driven profile switch")
}
func TestLogin_ProfileSwitchRejectsJWTObtainedUnderPreviousConfig(t *testing.T) {
s, _, _, username, cfgPath := setupServerWithProfile(t)
s.rootCtx = internal.CtxInitState(context.Background())
s.isLoginRequiredFn = func(context.Context) (bool, error) {
return false, errors.New("stop once the config is swapped")
}
other := "other-profile"
_, err := profilemanager.UpdateOrCreateConfig(profilemanager.ConfigInput{
ConfigPath: filepath.Join(filepath.Dir(cfgPath), other+".json"),
ManagementURL: "https://api.netbird.io:443",
})
require.NoError(t, err)
// getConfig loads the MDM policy right before Login swaps s.config, so the
// fetcher runs where a RequestJWTAuth racing the login would land: after the
// switch dropped the previous profile's state, while s.config still belongs
// to the previous profile. It caches a token under the generation current at
// that point, as WaitJWTToken would.
generation := s.jwtCache.currentGeneration()
fetcher := &jwtStoringMDMFetcher{cache: s.jwtCache, caller: unprivilegedIdentity()}
s.mdmLoader = mdm.NewLoader(fetcher)
_, err = s.Login(userCtx(), &proto.LoginRequest{ProfileName: &other, Username: &username})
require.Error(t, err)
require.Greater(t, fetcher.generation, generation, "the token was not cached after the switch dropped the previous profile's state")
_, found := s.jwtCache.get(fetcher.caller)
require.False(t, found, "a JWT obtained under the previous profile's config survived the login-driven switch")
}
func TestLogin_SameProfileKeepsPendingFlow(t *testing.T) {
s, _, profName, username, cfgPath := setupServerWithProfile(t)
s.rootCtx = internal.CtxInitState(context.Background())
s.isLoginRequiredFn = func(context.Context) (bool, error) {
return true, nil
}
breakProfilePrivateKey(t, cfgPath)
cancelled := false
flow := &stubOAuthFlow{}
s.oauthAuthFlow = oauthAuthFlow{
flow: flow,
hint: "user@example.com",
waitCancel: func() { cancelled = true },
}
_, err := s.Login(userCtx(), &proto.LoginRequest{ProfileName: &profName, Username: &username})
require.Error(t, err, "the broken key must stop the login before a flow is built")
require.Equal(t, flow, s.oauthAuthFlow.flow, "a login on the same profile dropped the flow a second client could join")
require.Equal(t, "user@example.com", s.oauthAuthFlow.hint)
require.False(t, cancelled, "a login on the same profile cancelled the pending wait")
}
func TestWaitSSOLogin_JudgesTheFlowThatProducedTheToken(t *testing.T) {
s := newSSOTestServer(t, "user@example.com", false, "user@example.com")
attempts := 0
s.loginAttemptFn = func(context.Context, string, string) (internal.StatusType, error) {
attempts++
return "", nil
}
flow := s.oauthAuthFlow.flow.(*stubOAuthFlow)
flow.onWait = func() {
s.mutex.Lock()
defer s.mutex.Unlock()
s.oauthAuthFlow.hint = "someone-else@example.com"
}
resp, err := s.WaitSSOLogin(callerCtx(t), &proto.WaitSSOLoginRequest{UserCode: "code"})
require.NoError(t, err, "a flow replaced mid-wait must not decide this wait's verdict")
require.NotNil(t, resp)
require.Equal(t, 1, attempts)
require.False(t, s.forceAccountPrompt, "the prompt was armed off another flow's hint")
}
func TestReuseOAuthFlow_ForcedPromptRefusesTheCachedFlow(t *testing.T) {
s := New(internal.CtxInitState(context.Background()), "console", "", false, false, false, false)
cancelled := false
s.oauthAuthFlow = oauthAuthFlow{
flow: &stubOAuthFlow{},
info: auth.AuthFlowInfo{UserCode: "code"},
expiresAt: time.Now().Add(time.Hour),
waitCancel: func() { cancelled = true },
}
state := internal.CtxGetState(s.rootCtx)
resp := s.reuseOAuthFlow(context.Background(), &stubOAuthFlow{}, state, true)
require.Nil(t, resp, "a forced account prompt reused the flow that skipped it")
require.True(t, cancelled, "the predecessor wait was orphaned")
resp = s.reuseOAuthFlow(context.Background(), &stubOAuthFlow{}, state, false)
require.NotNil(t, resp, "an unforced login stopped reusing a live flow")
require.Equal(t, "code", resp.UserCode)
}
func TestReplaceOAuthFlow_CancelsTheDisplacedWait(t *testing.T) {
s := New(internal.CtxInitState(context.Background()), "console", "", false, false, false, false)
cancelled := false
s.oauthAuthFlow = oauthAuthFlow{
flow: &stubOAuthFlow{},
info: auth.AuthFlowInfo{UserCode: "code"},
expiresAt: time.Now().Add(time.Hour),
hint: "user@example.com",
accountPrompted: true,
waitCancel: func() { cancelled = true },
}
next := &stubOAuthFlow{}
s.replaceOAuthFlow(oauthAuthFlow{flow: next, info: auth.AuthFlowInfo{UserCode: "next"}})
require.True(t, cancelled, "the displaced wait was left without an owner")
require.Equal(t, next, s.oauthAuthFlow.flow)
require.Equal(t, "next", s.oauthAuthFlow.info.UserCode)
require.Empty(t, s.oauthAuthFlow.hint, "the previous flow's hint survived the replacement")
require.False(t, s.oauthAuthFlow.accountPrompted)
require.Nil(t, s.oauthAuthFlow.waitCancel, "the consumed cancel stayed on the record")
}
func newSSOTestServer(t *testing.T, hint string, accountPrompted bool, tokenEmail string) *Server {
t.Helper()
s := New(internal.CtxInitState(context.Background()), "console", "", false, false, false, false)
s.oauthAuthFlow = oauthAuthFlow{
flow: &stubOAuthFlow{token: auth.TokenInfo{Email: tokenEmail, EmailClaim: tokenEmail}},
info: auth.AuthFlowInfo{UserCode: "code"},
expiresAt: time.Now().Add(time.Minute),
hint: hint,
accountPrompted: accountPrompted,
}
return s
}
// callerCtx is the gRPC caller's context. WaitSSOLogin parks a goroutine on it
// for the whole browser leg, so a test that never cancels leaks one.
func callerCtx(t *testing.T) context.Context {
t.Helper()
ctx, cancel := context.WithCancel(context.Background())
t.Cleanup(cancel)
return ctx
}
+1 -1
View File
@@ -93,7 +93,7 @@ func TestLogin_ChangeThatBecomesPrivilegedMidRequestHasNoSideEffects(t *testing.
require.NoError(t, err)
require.Equal(t, profilemanager.ID(activeProfile), active.ID, "the refused login switched the active profile anyway")
stored, err := profilemanager.ReadConfig(targetPath)
stored, err := profilemanager.GetExistingConfig(targetPath)
require.NoError(t, err)
require.Equal(t, "https://api.netbird.io:443", stored.ManagementURL.String(), "the refused login moved the management URL")
}
+6 -2
View File
@@ -7,6 +7,7 @@ import (
"github.com/stretchr/testify/require"
"github.com/netbirdio/netbird/client/internal/profilemanager"
"github.com/netbirdio/netbird/client/proto"
)
func TestPersistLoginOverrides(t *testing.T) {
@@ -80,10 +81,13 @@ func TestPersistLoginOverrides(t *testing.T) {
require.NoError(t, err, "seed config")
activeProf := &profilemanager.ActiveProfileState{ID: "default"}
err = persistLoginOverrides(activeProf, tt.newMgmtURL, tt.newPSK)
err = persistLoginOverrides(activeProf, &proto.LoginRequest{
ManagementUrl: tt.newMgmtURL,
OptionalPreSharedKey: tt.newPSK,
})
require.NoError(t, err, "persistLoginOverrides")
cfg, err := profilemanager.ReadConfig(profilemanager.DefaultConfigPath)
cfg, err := profilemanager.ReadConfigOrDefault(profilemanager.DefaultConfigPath)
require.NoError(t, err, "read back config")
require.Equal(t, tt.wantMgmtURL, cfg.ManagementURL.String(), "management URL")
+1 -1
View File
@@ -129,7 +129,7 @@ func TestLogout_ForeignUserProfileDoesNotUseTheRunningConfig(t *testing.T) {
// refused with PermissionDenied. The namesake profile does not, so the
// correct path gets as far as dialing its own unreachable management URL.
enableSSHOnProfile(t, cfgPath)
running, err := profilemanager.GetConfig(cfgPath)
running, err := profilemanager.GetExistingConfig(cfgPath)
require.NoError(t, err)
s.config = running
s.connectClient = newDummyConnectClient(context.Background())
-90
View File
@@ -182,96 +182,6 @@ func mdmManagedFieldConflicts(msg *proto.SetConfigRequest, policy *mdm.Policy) [
})
}
// setConfigRequestHasConfigOverrides reports whether the SetConfigRequest
// carries ANY field that would actually mutate the persisted config.
// The CLI builds a SetConfigRequest unconditionally on every
// `netbird up` (see setupSetConfigReq in cmd/up.go) — a plain
// `netbird up` produces a request with every field at its zero value;
// the gate must skip such no-op invocations or it would always fire
// even when the user did not pass any --flag. Returns false on a nil
// msg; true when any management/admin URL, PSK, DNS/NAT list+clean
// flag, interface/port/MTU, or any optional bool/duration field is set.
func setConfigRequestHasConfigOverrides(msg *proto.SetConfigRequest) bool {
if msg == nil {
return false
}
return msg.ManagementUrl != "" ||
msg.AdminURL != "" ||
msg.OptionalPreSharedKey != nil ||
len(msg.CustomDNSAddress) > 0 ||
len(msg.NatExternalIPs) > 0 || msg.CleanNATExternalIPs ||
len(msg.ExtraIFaceBlacklist) > 0 ||
len(msg.DnsLabels) > 0 || msg.CleanDNSLabels ||
msg.DnsRouteInterval != nil ||
msg.RosenpassEnabled != nil ||
msg.RosenpassPermissive != nil ||
msg.InterfaceName != nil ||
msg.WireguardPort != nil ||
msg.Mtu != nil ||
msg.DisableAutoConnect != nil ||
msg.ServerSSHAllowed != nil ||
msg.RemoteJobsAllowed != nil ||
msg.ServerVNCAllowed != nil ||
msg.DisableVNCApproval != nil ||
msg.NetworkMonitor != nil ||
msg.DisableClientRoutes != nil ||
msg.DisableServerRoutes != nil ||
msg.DisableDns != nil ||
msg.DisableFirewall != nil ||
msg.BlockLanAccess != nil ||
msg.DisableNotifications != nil ||
msg.BlockInbound != nil ||
msg.DisableIpv6 != nil ||
msg.EnableSSHRoot != nil ||
msg.EnableSSHSFTP != nil ||
msg.EnableSSHLocalPortForwarding != nil ||
msg.EnableSSHRemotePortForwarding != nil ||
msg.DisableSSHAuth != nil ||
msg.SshJWTCacheTTL != nil ||
msg.EnableLocalMetrics != nil ||
msg.LocalMetricsAddress != nil
}
// loginRequestHasConfigOverrides reports whether the LoginRequest
// carries ANY field that would mutate persisted daemon configuration
// (as opposed to pure-auth fields like setupKey, hostname, hint,
// profileName, username). Used by the Login handler to decide whether
// the `--disable-update-settings` / MDM gates must run: a re-auth that
// changes nothing about the configuration is always allowed.
func loginRequestHasConfigOverrides(msg *proto.LoginRequest) bool {
if msg == nil {
return false
}
return msg.ManagementUrl != "" ||
msg.AdminURL != "" ||
msg.PreSharedKey != "" || //nolint:staticcheck // SA1019: legacy proto field still accepted by Login
msg.OptionalPreSharedKey != nil ||
len(msg.CustomDNSAddress) > 0 ||
len(msg.NatExternalIPs) > 0 || msg.CleanNATExternalIPs ||
msg.RosenpassEnabled != nil ||
msg.InterfaceName != nil ||
msg.WireguardPort != nil ||
msg.DisableAutoConnect != nil ||
msg.ServerSSHAllowed != nil ||
msg.RemoteJobsAllowed != nil ||
msg.ServerVNCAllowed != nil ||
msg.DisableVNCApproval != nil ||
msg.RosenpassPermissive != nil ||
len(msg.ExtraIFaceBlacklist) > 0 ||
msg.NetworkMonitor != nil ||
msg.DnsRouteInterval != nil ||
msg.DisableClientRoutes != nil ||
msg.DisableServerRoutes != nil ||
msg.DisableDns != nil ||
msg.DisableFirewall != nil ||
msg.BlockLanAccess != nil ||
msg.DisableNotifications != nil ||
len(msg.DnsLabels) > 0 || msg.CleanDNSLabels ||
msg.BlockInbound != nil ||
msg.EnableLocalMetrics != nil ||
msg.LocalMetricsAddress != nil
}
// loginRequestMDMConflicts mirrors mdmManagedFieldConflicts but for the
// LoginRequest surface. Same value-aware semantics: a field set to the
// MDM-enforced value is a no-op echo, not a conflict; only a divergent
+59
View File
@@ -0,0 +1,59 @@
package server
import (
"os"
"path/filepath"
"testing"
"github.com/stretchr/testify/require"
"github.com/netbirdio/netbird/client/internal/profilemanager"
)
// The daemon provisions the peer's identity and persists it, because a key that
// stayed in memory would come back different on the next start and register a
// second peer. Provisioning is idempotent: a profile that already has an
// identity keeps the one on disk.
func TestProvisionProfileIdentity(t *testing.T) {
origDir := profilemanager.DefaultConfigPathDir
origPath := profilemanager.DefaultConfigPath
t.Cleanup(func() {
profilemanager.DefaultConfigPathDir = origDir
profilemanager.DefaultConfigPath = origPath
})
dir := t.TempDir()
profilemanager.DefaultConfigPathDir = dir
profilemanager.DefaultConfigPath = filepath.Join(dir, "default.json")
activeProf := &profilemanager.ActiveProfileState{ID: "default"}
t.Run("a profile with no file is provisioned and written", func(t *testing.T) {
_, err := os.Stat(profilemanager.DefaultConfigPath)
require.True(t, os.IsNotExist(err), "the fixture starts without a config file")
config, existed, err := provisionProfileIdentity(activeProf)
require.NoError(t, err)
require.False(t, existed, "the file was reported as pre-existing")
require.NotEmpty(t, config.PrivateKey)
stored, err := profilemanager.GetExistingConfig(profilemanager.DefaultConfigPath)
require.NoError(t, err, "provisioning did not write the config out")
require.Equal(t, config.PrivateKey, stored.PrivateKey, "the persisted identity is not the one returned")
require.NotEmpty(t, stored.SSHKey)
})
t.Run("a second call keeps the identity on disk", func(t *testing.T) {
before, err := profilemanager.GetExistingConfig(profilemanager.DefaultConfigPath)
require.NoError(t, err)
config, existed, err := provisionProfileIdentity(activeProf)
require.NoError(t, err)
require.True(t, existed)
require.Equal(t, before.PrivateKey, config.PrivateKey, "provisioning minted a second identity")
after, err := profilemanager.GetExistingConfig(profilemanager.DefaultConfigPath)
require.NoError(t, err)
require.Equal(t, before.PrivateKey, after.PrivateKey, "provisioning rewrote the stored identity")
})
}
+395 -159
View File
@@ -58,14 +58,23 @@ const (
// JWT token cache TTL for the client daemon (disabled by default)
defaultJWTCacheTTL = 0
errRestoreResidualState = "failed to restore residual state: %v"
errProfilesDisabled = "profiles are disabled, you cannot use this feature without profiles enabled"
errRestoreResidualState = "failed to restore residual state: %v"
errProfilesDisabled = "profiles are disabled, you cannot use this feature without profiles enabled"
// errUpdateSettingsDisabled is returned with codes.FailedPrecondition, not
// codes.Unavailable: the daemon answered, and it refused. Unavailable means
// "the daemon cannot serve this", which is why the CLI downgrades it to a
// warning and the GUI reads it as an unreachable daemon — both wrong for a
// refusal the caller has to act on.
errUpdateSettingsDisabled = "update settings are disabled, you cannot use this feature without update settings enabled"
errNetworksDisabled = "network selection is disabled by the administrator"
)
var ErrServiceNotUp = errors.New("service is not up")
type statusSetter interface {
Set(update internal.StatusType)
}
// Server for service control.
type Server struct {
rootCtx context.Context
@@ -83,6 +92,12 @@ type Server struct {
uiLogPath string
oauthAuthFlow oauthAuthFlow
// forceAccountPrompt makes the next startSSOLogin build its flow with a
// forced account prompt. Armed when a login came back for an account other
// than the hinted one: that flow's browser is gone, so the correction has to
// ride on the user's next connect. Guarded by mutex; deliberately not
// persisted — a lost flag only costs one more mismatch round.
forceAccountPrompt bool
// extendAuthSessionFlow holds the pending PKCE flow created by
// RequestExtendAuthSession until WaitExtendAuthSession resolves it.
// Kept separate from oauthAuthFlow (which is reserved for the SSH
@@ -176,6 +191,14 @@ type oauthAuthFlow struct {
cacheGeneration uint64
waitCancel context.CancelFunc
// hint is the account the flow was asked to sign in (login_hint). The token
// that comes back is compared against it; empty means nothing to compare.
hint string
// accountPrompted records that this flow already asked the IdP to re-decide
// the account (or could not ask — the device flow has no way to). A token
// for the wrong account is then let through with a warning instead of
// erroring again, so the flow cannot loop.
accountPrompted bool
}
// New server instance constructor.
@@ -496,16 +519,27 @@ func (s *Server) SetConfig(callerCtx context.Context, msg *proto.SetConfigReques
s.mutex.Lock()
defer s.mutex.Unlock()
// Skip the update-settings gate when the request carries no actual
// overrides: the CLI builds a SetConfigRequest unconditionally on
// every `netbird up` (setupSetConfigReq in cmd/up.go), so a plain
// `netbird up` would otherwise always trip the gate and surface a
// misleading "setConfig method is not available" warning, even when
// the user did not pass any config flag.
if setConfigRequestHasConfigOverrides(msg) {
if s.checkUpdateSettingsDisabled() {
return nil, gstatus.Errorf(codes.Unavailable, errUpdateSettingsDisabled)
}
stored, err := s.storedProfileConfig(msg.ProfileName, msg.Username)
if err != nil {
return nil, err
}
config, err := s.setConfigInputFromRequest(msg)
if err != nil {
return nil, err
}
// Update-settings gate: refuse the request only when it would actually
// change a persisted setting. The CLI builds a SetConfigRequest
// unconditionally on every `netbird up` (setupSetConfigReq in
// cmd/up.go) and fills it from its flags and environment, so a service
// or container that restates the configuration it already runs with
// must pass the gate. Deciding this on field presence alone refused
// those callers, and — through the identical gate in Login — refused
// their login too, which left a client configured by environment
// (NB_MANAGEMENT_URL and friends) unable to come up at all.
if s.checkUpdateSettingsDisabled() && configChangeRequested(stored, config) {
return nil, gstatus.Errorf(codes.FailedPrecondition, errUpdateSettingsDisabled)
}
// MDM gate: refuse the whole request if any of its fields is enforced
@@ -517,19 +551,10 @@ func (s *Server) SetConfig(callerCtx context.Context, msg *proto.SetConfigReques
return nil, err
}
stored, err := s.storedProfileConfig(msg.ProfileName, msg.Username)
if err != nil {
return nil, err
}
if err := requirePrivilegeForConfigChange(callerCtx, stored, privilegedChangeFromSetConfig(msg)); err != nil {
return nil, err
}
config, err := s.setConfigInputFromRequest(msg)
if err != nil {
return nil, err
}
updatedConf, err := profilemanager.UpdateConfig(config)
if err != nil {
log.Errorf("failed to update profile config: %v", err)
@@ -647,37 +672,45 @@ func (s *Server) setConfigInputFromRequest(msg *proto.SetConfigRequest) (profile
// Login uses setup key to prepare configuration for the daemon.
func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*proto.LoginResponse, error) {
activeProf, err := s.profileManager.GetActiveProfileState()
if err != nil {
return nil, fmt.Errorf("failed to get active profile state: %w", err)
}
// The stored config of the profile this request targets backs all three
// gates below. It is read before anything changes daemon state, so a
// refused login neither switches the profile nor cancels a login already
// in progress, and it is the profile the switch further down would
// activate.
stored, err := s.storedLoginConfig(activeProf, msg)
if err != nil {
return nil, err
}
// Config-override gates. LoginRequest carries the same surface as
// SetConfigRequest (managementUrl, PSK, ssh/rosenpass/port toggles,
// ...), so the same protections must apply. Without these the CLI
// command `netbird up --management-url=X` (which falls through to
// Login when SetConfig is rejected — see cmd/up.go) would silently
// bypass `--disable-update-settings` and any MDM policy.
if loginRequestHasConfigOverrides(msg) {
if s.checkUpdateSettingsDisabled() {
return nil, gstatus.Errorf(codes.Unavailable, errUpdateSettingsDisabled)
}
policy := s.mdmLoader.Load()
if err := rejectMDMManagedFieldConflicts(loginRequestMDMConflicts(msg, policy)); err != nil {
return nil, err
}
//
// The update-settings gate is value-aware, as in SetConfig: it looks at
// what a login would actually persist (loginOverridesInput) and refuses
// only a real divergence from the stored config. A login that restates
// the values already on disk changes nothing, so it must go through —
// that is what keeps a re-login, or a container restart carrying
// NB_MANAGEMENT_URL, working with the kill switch on.
if s.checkUpdateSettingsDisabled() && configChangeRequested(stored, loginOverridesInput(msg)) {
return nil, gstatus.Errorf(codes.FailedPrecondition, errUpdateSettingsDisabled)
}
activeProf, err := s.profileManager.GetActiveProfileState()
if err != nil {
log.Errorf("failed to get active profile state: %v", err)
return nil, fmt.Errorf("failed to get active profile state: %w", err)
policy := s.mdmLoader.Load()
if err := rejectMDMManagedFieldConflicts(loginRequestMDMConflicts(msg, policy)); err != nil {
return nil, err
}
// Privilege gate: same restrictions as SetConfig, since LoginRequest can carry
// the same fields. It runs before anything here changes daemon state, so a
// refused login neither switches the profile nor cancels a login already in
// progress, and it reads the profile the request targets, which is the one the
// switch below would activate.
stored, err := s.storedLoginConfig(activeProf, msg)
if err != nil {
return nil, err
}
// the same fields.
if err := requirePrivilegeForConfigChange(callerCtx, stored, privilegedChangeFromLogin(msg)); err != nil {
return nil, err
}
@@ -695,7 +728,7 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro
}
}()
ctx, activeProf, err := s.authorizeAndPrepareLogin(callerCtx, msg, activeProf)
ctx, activeProf, switched, err := s.authorizeAndPrepareLogin(callerCtx, msg, activeProf)
if err != nil {
// The RPC boundary is where this gets recorded: nothing logs handler
// errors for us, and a caller that retries would otherwise leave no
@@ -725,6 +758,9 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro
}
s.mutex.Lock()
s.config = config
if switched {
s.jwtCache.clear()
}
s.mutex.Unlock()
s.localMetrics.Reconcile(config.LocalMetricsEnabled, config.LocalMetricsAddress)
@@ -747,7 +783,7 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro
}
if msg.SetupKey == "" {
return s.beginSSOLogin(ctx, config, msg)
return s.startSSOLogin(ctx, msg, config, state)
}
// Setup-key path: we are about to dial Management with the key, so the
@@ -763,23 +799,31 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro
return &proto.LoginResponse{}, nil
}
// beginSSOLogin starts the browser leg of a login that carries no setup key and
// returns the response that parks the caller on it.
func (s *Server) beginSSOLogin(ctx context.Context, config *profilemanager.Config, msg *proto.LoginRequest) (*proto.LoginResponse, error) {
state := internal.CtxGetState(s.rootCtx)
// startSSOLogin opens the interactive leg of a login: it reuses the in-flight
// OAuth flow when one is still valid for the same client, and otherwise
// requests fresh auth info and parks the daemon on StatusNeedsLogin.
func (s *Server) startSSOLogin(ctx context.Context, msg *proto.LoginRequest, config *profilemanager.Config, state statusSetter) (*proto.LoginResponse, error) {
hint := ""
if msg.Hint != nil {
hint = *msg.Hint
}
oAuthFlow, err := auth.NewOAuthFlow(ctx, config, msg.IsUnixDesktopClient, false, hint)
oAuthFlow, err := auth.NewOAuthFlow(ctx, config, msg.IsUnixDesktopClient, false, hint, false)
if err != nil {
state.Set(internal.StatusLoginFailed)
return nil, err
}
if resp := s.pendingOAuthFlowResponse(ctx, oAuthFlow); resp != nil {
state.Set(internal.StatusNeedsLogin)
s.mutex.Lock()
promptForAccount := s.forceAccountPrompt
s.forceAccountPrompt = false
s.mutex.Unlock()
if promptForAccount && auth.RetryFlowForAccount(oAuthFlow) == nil {
// The device flow cannot ask; run it as-is. accountPrompted still goes
// true below so a second mismatch is let through instead of looping.
log.Warnf("the previous login returned a different account, but this flow cannot ask the IdP to choose one")
}
if resp := s.reuseOAuthFlow(ctx, oAuthFlow, state, promptForAccount); resp != nil {
return resp, nil
}
@@ -789,11 +833,13 @@ func (s *Server) beginSSOLogin(ctx context.Context, config *profilemanager.Confi
return nil, err
}
s.mutex.Lock()
s.oauthAuthFlow.flow = oAuthFlow
s.oauthAuthFlow.info = authInfo
s.oauthAuthFlow.expiresAt = time.Now().Add(time.Duration(authInfo.ExpiresIn) * time.Second)
s.mutex.Unlock()
s.replaceOAuthFlow(oauthAuthFlow{
flow: oAuthFlow,
info: authInfo,
expiresAt: time.Now().Add(time.Duration(authInfo.ExpiresIn) * time.Second),
hint: hint,
accountPrompted: promptForAccount,
})
state.Set(internal.StatusNeedsLogin)
@@ -805,32 +851,74 @@ func (s *Server) beginSSOLogin(ctx context.Context, config *profilemanager.Confi
}, nil
}
// pendingOAuthFlowResponse returns the in-flight flow's response when it
// targets the same IdP client and has enough time left for the user to finish
// the browser leg, so a second login joins the pending flow instead of opening
// a competing one. A flow too close to expiry has its waiter cancelled and nil
// returned, leaving the caller to start a fresh flow.
func (s *Server) pendingOAuthFlowResponse(ctx context.Context, oAuthFlow auth.OAuthFlow) *proto.LoginResponse {
if s.oauthAuthFlow.flow == nil || s.oauthAuthFlow.flow.GetClientID(ctx) != oAuthFlow.GetClientID(ctx) {
// replaceOAuthFlow installs next as the shared OAuth flow record and takes over
// the wait it displaces, so a WaitSSOLogin still parked on the old flow is not
// left without an owner: nothing would preempt it, and it could go on to run
// attemptLogin or mutate the record behind the new flow.
//
// The displaced cancel is read in the same critical section that replaces the
// record, so two callers racing here cannot both take the same predecessor. The
// cancel runs after the unlock — the displaced wait takes s.mutex as it unwinds.
func (s *Server) replaceOAuthFlow(next oauthAuthFlow) {
s.mutex.Lock()
staleCancel := s.oauthAuthFlow.waitCancel
s.oauthAuthFlow = next
s.mutex.Unlock()
if staleCancel != nil {
staleCancel()
}
}
func (s *Server) expireOAuthFlow(flow auth.OAuthFlow) {
s.mutex.Lock()
if s.oauthAuthFlow.flow == flow {
s.oauthAuthFlow.expiresAt = time.Now()
}
s.mutex.Unlock()
}
// reuseOAuthFlow returns the cached auth info when the previous flow targets
// the same client and still has enough life left, and otherwise cancels the
// stale wait and returns nil so the caller requests a fresh flow.
//
// promptForAccount rules reuse out: the cached flow was built without the
// account prompt, so handing its URL back would repeat the silent
// authorization that returned the wrong account — and with the flag already
// consumed, no later round would ask either. The predecessor's wait is still
// cancelled on the way out, so it is not orphaned on its device-code window.
//
// The whole decision runs off one snapshot taken under s.mutex: a concurrent
// WaitSSOLogin replaces waitCancel and expires the flow, so reading the fields
// one at a time could cancel a wait that no longer belongs to the flow just
// judged stale, or answer with auth info from a flow that was already replaced.
// The cancel itself is called after unlocking — it runs arbitrary teardown, and
// WaitSSOLogin takes s.mutex on the way out.
func (s *Server) reuseOAuthFlow(ctx context.Context, oAuthFlow auth.OAuthFlow, state statusSetter, promptForAccount bool) *proto.LoginResponse {
s.mutex.Lock()
current := s.oauthAuthFlow
s.mutex.Unlock()
if current.flow == nil || current.flow.GetClientID(ctx) != oAuthFlow.GetClientID(ctx) {
return nil
}
if s.oauthAuthFlow.expiresAt.After(time.Now().Add(90 * time.Second)) {
log.Debugf("using previous oauth flow info")
return &proto.LoginResponse{
NeedsSSOLogin: true,
VerificationURI: s.oauthAuthFlow.info.VerificationURI,
VerificationURIComplete: s.oauthAuthFlow.info.VerificationURIComplete,
UserCode: s.oauthAuthFlow.info.UserCode,
if promptForAccount || !current.expiresAt.After(time.Now().Add(90*time.Second)) {
log.Warnf("canceling previous waiting execution")
if current.waitCancel != nil {
current.waitCancel()
}
return nil
}
log.Warnf("canceling previous waiting execution")
if s.oauthAuthFlow.waitCancel != nil {
s.oauthAuthFlow.waitCancel()
log.Debugf("using previous oauth flow info")
state.Set(internal.StatusNeedsLogin)
return &proto.LoginResponse{
NeedsSSOLogin: true,
VerificationURI: current.info.VerificationURI,
VerificationURIComplete: current.info.VerificationURIComplete,
UserCode: current.info.UserCode,
}
return nil
}
// WaitSSOLogin validates the supplied userCode against the in-flight OAuth
@@ -903,9 +991,16 @@ func (s *Server) WaitSSOLogin(callerCtx context.Context, msg *proto.WaitSSOLogin
}
s.actCancel = cancel
// One snapshot of the flow this wait belongs to. hint and accountPrompted
// are judged against the token that comes back below, and WaitToken blocks
// for the whole browser leg: a concurrent Login or RequestJWTAuth replaces
// s.oauthAuthFlow meanwhile, so re-reading them after the wait would judge
// this flow's token against another flow's account.
pending := s.oauthAuthFlow
flow := pending.flow
s.mutex.Unlock()
if s.oauthAuthFlow.flow == nil {
if flow == nil {
return nil, gstatus.Errorf(codes.Internal, "oauth flow is not initialized")
}
@@ -923,31 +1018,36 @@ func (s *Server) WaitSSOLogin(callerCtx context.Context, msg *proto.WaitSSOLogin
// the affordance instead of a Connecting that never resolves.
state.Set(internal.StatusNeedsLogin)
s.mutex.Lock()
flowInfo := s.oauthAuthFlow.info
s.mutex.Unlock()
flowInfo := pending.info
if flowInfo.UserCode != msg.UserCode {
state.Set(internal.StatusLoginFailed)
return nil, gstatus.Errorf(codes.InvalidArgument, "sso user code is invalid")
}
if s.oauthAuthFlow.waitCancel != nil {
s.oauthAuthFlow.waitCancel()
}
waitCTX, cancel := context.WithCancel(ctx)
defer cancel()
// Swap in this wait's cancel and take over the one it displaces in a single
// critical section, so two WaitSSOLogin calls racing here cannot both read
// the same predecessor and leave one wait uncancelled. Cancelling happens
// after the unlock: the displaced wait takes s.mutex as it unwinds.
s.mutex.Lock()
if s.oauthAuthFlow.flow != flow {
s.mutex.Unlock()
return nil, gstatus.Errorf(codes.Canceled, "sso login was replaced by a newer login")
}
staleCancel := s.oauthAuthFlow.waitCancel
s.oauthAuthFlow.waitCancel = cancel
s.mutex.Unlock()
tokenInfo, err := s.oauthAuthFlow.flow.WaitToken(waitCTX, flowInfo)
if staleCancel != nil {
staleCancel()
}
tokenInfo, err := flow.WaitToken(waitCTX, flowInfo)
if err != nil {
s.mutex.Lock()
s.oauthAuthFlow.expiresAt = time.Now()
s.mutex.Unlock()
s.expireOAuthFlow(flow)
switch {
case errors.Is(err, context.Canceled):
// External abort. If our caller cancelled (the client closed
@@ -960,7 +1060,9 @@ func (s *Server) WaitSSOLogin(callerCtx context.Context, msg *proto.WaitSSOLogin
// the new owner — don't clobber it.
if callerCtx.Err() != nil {
s.mutex.Lock()
s.oauthAuthFlow = oauthAuthFlow{}
if s.oauthAuthFlow.flow == flow {
s.oauthAuthFlow = oauthAuthFlow{}
}
s.mutex.Unlock()
}
case errors.Is(err, context.DeadlineExceeded):
@@ -976,9 +1078,32 @@ func (s *Server) WaitSSOLogin(callerCtx context.Context, msg *proto.WaitSSOLogin
return nil, err
}
s.mutex.Lock()
s.oauthAuthFlow.expiresAt = time.Now()
s.mutex.Unlock()
s.expireOAuthFlow(flow)
if !tokenInfo.MatchesAccount(pending.hint) {
if !pending.accountPrompted {
// The IdP answered from a session belonging to another account. The
// browser for this flow is gone, so a new URL cannot be handed out
// here — arm the prompt for the user's next connect and fail this
// round. Never log in with the token: on a registered peer the
// server would reject it, and on a fresh one it would silently
// register the peer under the wrong account.
log.Warnf("login returned an account other than the one this profile is bound to; the next connect will ask the IdP to choose")
s.mutex.Lock()
if s.oauthAuthFlow.flow == flow {
s.oauthAuthFlow = oauthAuthFlow{}
s.forceAccountPrompt = true
}
s.mutex.Unlock()
state.Set(internal.StatusNeedsLogin)
return nil, gstatus.Errorf(codes.FailedPrecondition, "the login used a different account than this profile; connect again to choose the account")
}
// Already asked once; the account may legitimately differ (a changed
// email address). Refusing again would lock the user out of the profile,
// and the management server still rejects a token that does not own the
// peer.
log.Warnf("login still returned a different account after the prompt, continuing with it")
}
if loginStatus, err := s.attemptLogin(ctx, "", tokenInfo.GetTokenToUse()); err != nil {
state.Set(loginStatus)
@@ -1076,11 +1201,15 @@ func (s *Server) Up(callerCtx context.Context, msg *proto.UpRequest) (*proto.UpR
}
if msg != nil && msg.ProfileName != nil {
if _, err := s.switchProfileIfNeeded(*msg.ProfileName, msg.Username, activeProf); err != nil {
switched, err := s.switchProfileIfNeeded(*msg.ProfileName, msg.Username, activeProf)
if err != nil {
s.mutex.Unlock()
log.Errorf("failed to switch profile: %v", err)
return nil, err
}
if switched {
s.dropPendingAuthFlows()
}
}
activeProf, err = s.profileManager.GetActiveProfileState()
@@ -1181,12 +1310,16 @@ func (s *Server) storedLoginConfig(activeProf *profilemanager.ActiveProfileState
// storedConfigAtPath reads a profile config file with the MDM policy applied on
// top, yielding nil when the file does not exist yet.
//
// The overlay matters because the privilege gates are this function's only
// consumers, and they have to reason about the config the engine will actually
// run. An MDM policy that enables a remote-access server leaves the on-disk
// value off, so without it the guards read a host publishing its console as one
// running nothing, and let an unprivileged caller repoint the management
// identity that decides who may connect to it.
// The overlay matters because the privilege gates have to reason about the
// config the engine will actually run. An MDM policy that enables a
// remote-access server leaves the on-disk value off, so without it the guards
// read a host publishing its console as one running nothing, and let an
// unprivileged caller repoint the management identity that decides who may
// connect to it.
//
// Reading it has no side effect: profilemanager.GetExistingConfig does not
// write, so a request that the gates go on to refuse leaves the profile file as
// it found it.
func (s *Server) storedConfigAtPath(path string) (*profilemanager.Config, error) {
if _, err := os.Stat(path); err != nil {
if os.IsNotExist(err) {
@@ -1195,7 +1328,7 @@ func (s *Server) storedConfigAtPath(path string) (*profilemanager.Config, error)
return nil, fmt.Errorf("stat profile config: %w", err)
}
cfg, err := profilemanager.GetConfig(path)
cfg, err := profilemanager.GetExistingConfig(path)
if err != nil {
return nil, fmt.Errorf("read profile config: %w", err)
}
@@ -1222,12 +1355,12 @@ func (s *Server) resolveProfileHandle(handle, username string) (*profilemanager.
}
// switchProfileIfNeeded resolves the user-supplied handle, updates the
// active profile state if it differs from the current one, and returns
// the resolved profile so callers can include its ID in RPC responses.
func (s *Server) switchProfileIfNeeded(handle string, userName *string, activeProf *profilemanager.ActiveProfileState) (*profilemanager.Profile, error) {
// active profile state if it differs from the current one, and reports
// whether the active profile changed.
func (s *Server) switchProfileIfNeeded(handle string, userName *string, activeProf *profilemanager.ActiveProfileState) (bool, error) {
if handle != profilemanager.DefaultProfileName && (userName == nil || *userName == "") {
log.Errorf("profile name is set to %s, but username is not provided", handle)
return nil, fmt.Errorf("profile name is set to %s, but username is not provided", handle)
return false, fmt.Errorf("profile name is set to %s, but username is not provided", handle)
}
var username string
@@ -1237,26 +1370,48 @@ func (s *Server) switchProfileIfNeeded(handle string, userName *string, activePr
resolved, err := s.resolveProfileHandle(handle, username)
if err != nil {
return nil, err
return false, err
}
if resolved.ID != activeProf.ID || username != activeProf.Username {
if s.checkProfilesDisabled() {
log.Errorf("profiles are disabled, you cannot use this feature without profiles enabled")
return nil, gstatus.Errorf(codes.Unavailable, errProfilesDisabled)
}
log.Infof("switching to profile %s (%s) for user %s", resolved.Name, resolved.ID, username)
if err := s.profileManager.SetActiveProfileState(&profilemanager.ActiveProfileState{
ID: resolved.ID,
Username: username,
}); err != nil {
log.Errorf("failed to set active profile state: %v", err)
return nil, fmt.Errorf("failed to set active profile state: %w", err)
}
if resolved.ID == activeProf.ID && username == activeProf.Username {
return false, nil
}
return resolved, nil
if s.checkProfilesDisabled() {
log.Errorf("profiles are disabled, you cannot use this feature without profiles enabled")
return false, gstatus.Errorf(codes.Unavailable, errProfilesDisabled)
}
log.Infof("switching to profile %s (%s) for user %s", resolved.Name, resolved.ID, username)
if err := s.profileManager.SetActiveProfileState(&profilemanager.ActiveProfileState{
ID: resolved.ID,
Username: username,
}); err != nil {
log.Errorf("failed to set active profile state: %v", err)
return false, fmt.Errorf("failed to set active profile state: %w", err)
}
return true, nil
}
func (s *Server) dropPendingAuthFlows() {
// A pending login flow and the account-prompt flag describe the previous
// profile's login; carried across a switch they would judge the new
// profile's token against the old profile's account. CancelFunc is
// non-blocking, so calling it under the mutex is safe.
if cancel := s.oauthAuthFlow.waitCancel; cancel != nil {
cancel()
}
s.oauthAuthFlow = oauthAuthFlow{}
s.forceAccountPrompt = false
// A pending session extend belongs to the previous profile too: its device
// code was issued by that profile's IdP client, and WaitExtendAuthSession
// would submit the resulting token against the new profile's engine.
s.extendAuthSessionFlow.CancelWait()
s.extendAuthSessionFlow.Clear()
s.jwtCache.clear()
}
// SwitchProfile switches the active profile in the daemon.
@@ -1290,7 +1445,7 @@ func (s *Server) SwitchProfile(callerCtx context.Context, msg *proto.SwitchProfi
s.config = config
s.localMetrics.Reconcile(config.LocalMetricsEnabled, config.LocalMetricsAddress)
s.jwtCache.clear()
s.dropPendingAuthFlows()
if msg != nil && msg.ProfileName != nil {
s.publishProfileListChanged(*msg.ProfileName)
@@ -1499,8 +1654,16 @@ func (s *Server) handleActiveProfileLogout(ctx context.Context) (*proto.LogoutRe
return &proto.LogoutResponse{}, nil
}
// getConfig reads config file and returns Config and whether the config file already existed. Errors out if it does not exist
func (s *Server) getConfig(activeProf *profilemanager.ActiveProfileState) (*profilemanager.Config, bool, error) {
// provisionProfileIdentity resolves the active profile's config and puts the
// keys that identify the peer on disk, reporting whether the config file
// already existed.
//
// This is the daemon's provisioning point: the config resolved here is the one
// the peer runs with, so it needs its identity, and that has to reach disk — a
// key that stays in memory would come back different on the next start and
// re-register the peer. Reads themselves are pure, so the write is here, in
// the open, instead of hiding inside the reader.
func provisionProfileIdentity(activeProf *profilemanager.ActiveProfileState) (*profilemanager.Config, bool, error) {
cfgPath, err := activeProf.FilePath()
if err != nil {
return nil, false, fmt.Errorf("failed to get active profile file path: %w", err)
@@ -1511,15 +1674,38 @@ func (s *Server) getConfig(activeProf *profilemanager.ActiveProfileState) (*prof
log.Infof("active profile config existed: %t, err %v", configExisted, err)
config, err := profilemanager.ReadConfig(cfgPath)
config, err := profilemanager.ReadConfigOrDefault(cfgPath)
if err != nil {
return nil, false, fmt.Errorf("failed to get config: %w", err)
}
// Apply the daemon-owned MDM policy on top of the just-resolved
// Config. profilemanager's apply() initialises the policy to
// empty — the Loader lives outside Config, so this overlay step
// is driven externally here.
generated, err := config.EnsureIdentity()
if err != nil {
return nil, false, fmt.Errorf("ensure profile identity: %w", err)
}
if generated || !configExisted {
if err := profilemanager.WriteOutConfig(cfgPath, config); err != nil {
return nil, false, fmt.Errorf("write out profile config: %w", err)
}
}
return config, configExisted, nil
}
// getConfig resolves the active profile's config, provisions its identity and
// reports whether the config file already existed.
func (s *Server) getConfig(activeProf *profilemanager.ActiveProfileState) (*profilemanager.Config, bool, error) {
config, configExisted, err := provisionProfileIdentity(activeProf)
if err != nil {
return nil, false, err
}
// Apply the daemon-owned MDM policy on top of the just-resolved Config.
// profilemanager's apply() initialises the policy to empty — the Loader
// lives outside Config, so this overlay step is driven externally here.
// After the write above, on purpose: the overlay is runtime-only and
// re-derived on every load, so the file keeps the profile's own values.
config.ApplyMDMPolicy(s.mdmLoader.Load())
return config, configExisted, nil
@@ -1574,7 +1760,7 @@ func (s *Server) logoutFromProfile(ctx context.Context, profile *profilemanager.
cfgPath = profilemanager.DefaultConfigPath
}
config, err := profilemanager.GetConfig(cfgPath)
config, err := profilemanager.GetExistingConfig(cfgPath)
if err != nil {
return fmt.Errorf("profile '%s' not found", profile.ID)
}
@@ -1593,6 +1779,19 @@ func (s *Server) sendLogoutRequestWithConfig(ctx context.Context, config *profil
// Privilege gate: deregistering frees this machine's key to be registered
// against another management server, which is only restricted while the SSH
// server makes that a privilege handover.
// Ahead of the privilege gate on purpose. A profile with no identity was
// never registered — a logout clears the keys in place, so logging the same
// profile out twice lands here — so there is nothing to deregister and
// nothing for the gate to protect: what it guards against is handing this
// machine's registered key to another management server. Behind the gate,
// an unprivileged caller would be refused instead, and for a profile whose
// ServerSSHAllowed is unset that is every caller, since an absent value
// counts as SSH enabled.
if config.PrivateKey == "" {
log.Infof("profile carries no identity, nothing to deregister")
return nil
}
if err := requirePrivilegeForDeregistration(ctx, config); err != nil {
return err
}
@@ -1924,7 +2123,7 @@ func (s *Server) RequestJWTAuth(
}
// 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)
oAuthFlow, err := auth.NewOAuthFlow(ctx, config, msg.GetHasGraphicalSession(), false, hint, false)
if err != nil {
return nil, gstatus.Errorf(codes.Internal, "failed to create OAuth flow: %v", err)
}
@@ -1934,12 +2133,15 @@ func (s *Server) RequestJWTAuth(
return nil, gstatus.Errorf(codes.Internal, "failed to request auth info: %v", err)
}
s.mutex.Lock()
s.oauthAuthFlow.flow = oAuthFlow
s.oauthAuthFlow.info = authInfo
s.oauthAuthFlow.expiresAt = time.Now().Add(time.Duration(authInfo.ExpiresIn) * time.Second)
s.oauthAuthFlow.cacheGeneration = cacheGeneration
s.mutex.Unlock()
// This flow carries no profile hint: leaving the previous login's hint and
// accountPrompted in place would have WaitSSOLogin judge a later token
// against them.
s.replaceOAuthFlow(oauthAuthFlow{
flow: oAuthFlow,
info: authInfo,
expiresAt: time.Now().Add(time.Duration(authInfo.ExpiresIn) * time.Second),
cacheGeneration: cacheGeneration,
})
return &proto.RequestJWTAuthResponse{
VerificationURI: authInfo.VerificationURI,
@@ -2039,7 +2241,7 @@ func (s *Server) RequestExtendAuthSession(
}
// 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)
oAuthFlow, err := auth.NewOAuthFlow(ctx, config, msg.GetHasGraphicalSession(), false, hint, true)
if err != nil {
return nil, gstatus.Errorf(codes.Internal, "failed to create OAuth flow: %v", err)
}
@@ -2284,7 +2486,7 @@ func (s *Server) GetConfig(ctx context.Context, req *proto.GetConfigRequest) (*p
cfgPath = profilemanager.DefaultConfigPath
}
cfg, err := profilemanager.GetConfig(cfgPath)
cfg, err := profilemanager.GetExistingConfig(cfgPath)
if err != nil {
log.Errorf("failed to get active profile config: %v", err)
return nil, fmt.Errorf("failed to get active profile config: %w", err)
@@ -2605,9 +2807,23 @@ func (s *Server) checkDisableAdvancedView() *bool {
return nil
}
// profileOwnerOption passes the OS account of the active profile to the connect client,
// which reads that account's certificate store for user certificate posture checks.
func (s *Server) profileOwnerOption() []internal.ConnectClientOption {
if s.profileManager == nil {
return nil
}
activeProf, err := s.profileManager.GetActiveProfileState()
if err != nil {
log.Warnf("failed to read the active profile owner, no user certificate store is used for certificate posture: %v", err)
return []internal.ConnectClientOption{internal.WithUnknownProfileOwner()}
}
return []internal.ConnectClientOption{internal.WithProfileOwner(activeProf.Username)}
}
func (s *Server) connect(ctx context.Context, config *profilemanager.Config, statusRecorder *peer.Status, runningChan chan struct{}) error {
log.Tracef("running client connection")
client := internal.NewConnectClient(ctx, config, statusRecorder)
client := internal.NewConnectClient(ctx, config, statusRecorder, s.profileOwnerOption()...)
client.SetUpdateManager(s.updateManager)
client.SetSyncResponsePersistence(s.persistSyncResponse)
@@ -2749,8 +2965,6 @@ func sendTerminalNotification() error {
return wallCmd.Wait()
}
// persistLoginOverrides writes management URL and pre-shared key from a LoginRequest to the
// active profile config so that subsequent reads pick them up. Empty/nil values are ignored.
// afterLoginPreCheck is a seam for tests to run a concurrent config change
// between Login's first privilege check and the authoritative one.
var afterLoginPreCheck func()
@@ -2765,7 +2979,7 @@ var afterLoginPreCheck func()
// of this is reached; this one exists because that check is not synchronized
// against a concurrent privileged request that enables the SSH server, and a
// caller refused here must not have cancelled or switched anything either.
func (s *Server) authorizeAndPrepareLogin(callerCtx context.Context, msg *proto.LoginRequest, activeProf *profilemanager.ActiveProfileState) (context.Context, *profilemanager.ActiveProfileState, error) {
func (s *Server) authorizeAndPrepareLogin(callerCtx context.Context, msg *proto.LoginRequest, activeProf *profilemanager.ActiveProfileState) (context.Context, *profilemanager.ActiveProfileState, bool, error) {
if afterLoginPreCheck != nil {
afterLoginPreCheck()
}
@@ -2775,10 +2989,19 @@ func (s *Server) authorizeAndPrepareLogin(callerCtx context.Context, msg *proto.
stored, err := s.storedLoginConfig(activeProf, msg)
if err != nil {
return nil, nil, err
return nil, nil, false, err
}
if err := requirePrivilegeForConfigChange(callerCtx, stored, privilegedChangeFromLogin(msg)); err != nil {
return nil, nil, err
return nil, nil, false, err
}
// The update-settings decision is re-taken here for the same reason as the
// privilege one: Login's earlier check ran outside this lock, so the stored
// config it compared against could have moved since. This one is the
// authoritative check, and it is the last read before persistLoginOverrides
// writes.
if s.checkUpdateSettingsDisabled() && configChangeRequested(stored, loginOverridesInput(msg)) {
return nil, nil, false, gstatus.Errorf(codes.FailedPrecondition, errUpdateSettingsDisabled)
}
s.mutex.Lock()
@@ -2796,29 +3019,46 @@ func (s *Server) authorizeAndPrepareLogin(callerCtx context.Context, msg *proto.
log.Warnf(errRestoreResidualState, err)
}
switched := false
if msg.ProfileName != nil {
if _, err := s.switchProfileIfNeeded(*msg.ProfileName, msg.Username, activeProf); err != nil {
return nil, nil, fmt.Errorf("switch profile: %w", err)
switched, err = s.switchProfileIfNeeded(*msg.ProfileName, msg.Username, activeProf)
if err != nil {
return nil, nil, false, fmt.Errorf("switch profile: %w", err)
}
if switched {
s.mutex.Lock()
s.dropPendingAuthFlows()
s.mutex.Unlock()
}
}
activeProf, err = s.profileManager.GetActiveProfileState()
if err != nil {
return nil, nil, fmt.Errorf("active profile state: %w", err)
return nil, nil, false, fmt.Errorf("active profile state: %w", err)
}
if err := persistLoginOverrides(activeProf, msg.ManagementUrl, msg.OptionalPreSharedKey); err != nil {
return nil, nil, fmt.Errorf("persist login overrides: %w", err)
if err := persistLoginOverrides(activeProf, msg); err != nil {
return nil, nil, false, fmt.Errorf("persist login overrides: %w", err)
}
return ctx, activeProf, nil
// Provisioning under the same lock as the decision above, and next to the
// write it guards. getConfig would otherwise mint the identity and persist
// it once this returns: between its read and its write, a SetConfig that
// had already answered its caller would be overwritten by the config this
// login read before it landed.
if _, _, err := provisionProfileIdentity(activeProf); err != nil {
return nil, nil, false, err
}
return ctx, activeProf, switched, nil
}
func persistLoginOverrides(activeProf *profilemanager.ActiveProfileState, managementURL string, preSharedKey *string) error {
if preSharedKey != nil && *preSharedKey == "" {
preSharedKey = nil
}
if managementURL == "" && preSharedKey == nil {
// persistLoginOverrides writes the config fields a login request is allowed to
// carry into the active profile. It shares its input builder with the
// update-settings gate, so the gate judges exactly the fields this writes.
func persistLoginOverrides(activeProf *profilemanager.ActiveProfileState, msg *proto.LoginRequest) error {
input := loginOverridesInput(msg)
if input.ManagementURL == "" && input.PreSharedKey == nil {
return nil
}
@@ -2827,11 +3067,7 @@ func persistLoginOverrides(activeProf *profilemanager.ActiveProfileState, manage
return fmt.Errorf("active profile file path: %w", err)
}
input := profilemanager.ConfigInput{
ConfigPath: cfgPath,
ManagementURL: managementURL,
PreSharedKey: preSharedKey,
}
input.ConfigPath = cfgPath
if _, err := profilemanager.UpdateOrCreateConfig(input); err != nil {
return fmt.Errorf("update config: %w", err)
}
+3 -1
View File
@@ -10,6 +10,7 @@ import (
"github.com/stretchr/testify/require"
"github.com/netbirdio/netbird/client/internal"
"github.com/netbirdio/netbird/client/internal/auth"
"github.com/netbirdio/netbird/client/internal/peer"
"github.com/netbirdio/netbird/client/proto"
)
@@ -21,7 +22,8 @@ func newTestServer() *Server {
// New always populates the SSH JWT cache and the logout and
// profile-switch paths call into it unconditionally, so a Server
// assembled field by field has to populate it too.
jwtCache: newJWTCache(),
jwtCache: newJWTCache(),
extendAuthSessionFlow: auth.NewPendingFlow(),
}
}
+3 -4
View File
@@ -10,9 +10,9 @@ import (
"testing"
"time"
"go.uber.org/mock/gomock"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/otel"
"go.uber.org/mock/gomock"
"github.com/netbirdio/netbird/management/server/integrations/integrated_validator/validator"
@@ -36,7 +36,6 @@ import (
"github.com/netbirdio/netbird/management/server"
"github.com/netbirdio/netbird/management/server/activity"
nbcache "github.com/netbirdio/netbird/management/server/cache"
"github.com/netbirdio/netbird/management/server/integrations/port_forwarding"
"github.com/netbirdio/netbird/management/server/permissions"
"github.com/netbirdio/netbird/management/server/settings"
"github.com/netbirdio/netbird/management/server/store"
@@ -200,8 +199,8 @@ func startManagement(t *testing.T, signalAddr string, counter *int) (*grpc.Serve
requestBuffer := server.NewAccountRequestBuffer(context.Background(), store)
peersUpdateManager := update_channel.NewPeersUpdateManager(metrics)
networkMapController := controller.NewController(context.Background(), store, metrics, peersUpdateManager, requestBuffer, server.MockIntegratedValidator{}, settingsMockManager, "netbird.selfhosted", port_forwarding.NewControllerMock(), manager.NewEphemeralManager(store, peersManager), config, nil)
accountManager, err := server.BuildManager(context.Background(), config, store, networkMapController, jobManager, nil, "", eventStore, nil, false, ia, metrics, port_forwarding.NewControllerMock(), settingsMockManager, permissionsManagerMock, false, cacheStore)
networkMapController := controller.NewController(context.Background(), store, metrics, peersUpdateManager, requestBuffer, server.MockIntegratedValidator{}, settingsMockManager, "netbird.selfhosted", manager.NewEphemeralManager(store, peersManager), config, nil)
accountManager, err := server.BuildManager(context.Background(), config, store, networkMapController, jobManager, nil, "", eventStore, nil, false, ia, metrics, settingsMockManager, permissionsManagerMock, false, cacheStore)
if err != nil {
return nil, "", err
}
+1 -1
View File
@@ -313,7 +313,7 @@ func TestSetConfig_MDMReject_AllOrNothing(t *testing.T) {
// Confirm RosenpassEnabled was NOT applied even though it was not
// in the conflict list: the request was rejected as a whole.
reloaded, err := profilemanager.GetConfig(cfgPath)
reloaded, err := profilemanager.GetExistingConfig(cfgPath)
require.NoError(t, err)
assert.False(t, reloaded.RosenpassEnabled, "non-conflicting field must not be applied when request is rejected")
}
+1 -1
View File
@@ -129,7 +129,7 @@ func TestSetConfig_AllFieldsSaved(t *testing.T) {
cfgPath, err := profState.FilePath()
require.NoError(t, err)
cfg, err := profilemanager.GetConfig(cfgPath)
cfg, err := profilemanager.GetExistingConfig(cfgPath)
require.NoError(t, err)
require.Equal(t, "https://new-api.netbird.io:443", cfg.ManagementURL.String())
+1 -17
View File
@@ -394,21 +394,5 @@ func sameManagementURL(stored *url.URL, requested string) bool {
return false
}
return stored.Scheme == parsed.Scheme &&
stored.Hostname() == parsed.Hostname() &&
effectivePort(stored) == effectivePort(parsed)
}
func effectivePort(u *url.URL) string {
if port := u.Port(); port != "" {
return port
}
switch u.Scheme {
case "https":
return "443"
case "http":
return "80"
default:
return ""
}
return profilemanager.SameServiceURL(stored, parsed)
}
+55
View File
@@ -0,0 +1,55 @@
package server
import (
log "github.com/sirupsen/logrus"
"github.com/netbirdio/netbird/client/internal/profilemanager"
"github.com/netbirdio/netbird/client/proto"
)
// configChangeRequested reports whether applying input would move the target
// profile away from the configuration it already persists. It is the decision
// procedure of the update-settings kill switch (--disable-update-settings /
// NB_DISABLE_UPDATE_SETTINGS / the MDM DisableUpdateSettings key): that switch
// forbids *changing* settings, so a request that restates the stored values is
// not a change and must not be refused.
//
// This has to be judged on values, not on field presence. `netbird up` rebuilds
// the whole config surface of SetConfigRequest and LoginRequest from its flags
// and environment on every invocation, so a service or container configured by
// environment restates its own configuration on every start. A presence-based
// gate refused those requests, and because Login carries the same fields it
// refused the login too — leaving such a client unable to come up at all.
//
// A dry run that cannot be evaluated fails closed: the request counts as a
// change, so a malformed field can never open the gate. The error itself is
// reported to the caller by the real update path.
func configChangeRequested(stored *profilemanager.Config, input profilemanager.ConfigInput) bool {
changed, err := stored.WouldChange(input)
if err != nil {
log.Warnf("cannot evaluate the requested config change, treating it as a change: %v", err)
return true
}
return changed
}
// loginOverridesInput builds the ConfigInput a login request persists. The
// management URL and the pre-shared key are the only config fields the daemon
// applies from a LoginRequest; everything else on that message is either pure
// auth or ignored. An empty pre-shared key is dropped rather than written, so
// a login cannot clear the stored key by omission.
//
// Both the write (persistLoginOverrides) and the update-settings gate go
// through this builder, so the gate can neither refuse a field the write
// ignores nor miss one it applies.
func loginOverridesInput(msg *proto.LoginRequest) profilemanager.ConfigInput {
preSharedKey := msg.OptionalPreSharedKey
if preSharedKey != nil && *preSharedKey == "" {
preSharedKey = nil
}
return profilemanager.ConfigInput{
ManagementURL: msg.ManagementUrl,
PreSharedKey: preSharedKey,
}
}
+390
View File
@@ -0,0 +1,390 @@
package server
import (
"context"
"os"
"path/filepath"
"testing"
"github.com/stretchr/testify/require"
"google.golang.org/grpc/codes"
gstatus "google.golang.org/grpc/status"
"github.com/netbirdio/netbird/client/internal"
"github.com/netbirdio/netbird/client/internal/profilemanager"
"github.com/netbirdio/netbird/client/mdm"
"github.com/netbirdio/netbird/client/proto"
)
// The seeded profile of setupServerWithProfile is created with this management
// URL, so a request carrying it restates what the profile already holds.
const storedManagementURL = "https://api.netbird.io:443"
// A client configured by environment re-sends its whole configuration on every
// `netbird up`: the CLI fills the request from its flags and env regardless of
// what changed. With the update-settings kill switch on, such a request must
// pass — nothing about the configuration moves.
func TestSetConfig_RestatingTheStoredConfigPassesTheGate(t *testing.T) {
s, ctx, profName, username, _ := setupServerWithProfile(t)
s.updateSettingsDisabled = true
_, err := s.SetConfig(ctx, &proto.SetConfigRequest{
ProfileName: profName,
Username: username,
ManagementUrl: storedManagementURL,
})
require.NoError(t, err, "restating the stored management URL is not a settings change")
}
// The same endpoint written without its default port is the same endpoint. A
// gate that compared raw strings refused NB_MANAGEMENT_URL=https://host, which
// is how the URL is normally spelled.
func TestSetConfig_EquivalentManagementURLPassesTheGate(t *testing.T) {
s, ctx, profName, username, _ := setupServerWithProfile(t)
s.updateSettingsDisabled = true
_, err := s.SetConfig(ctx, &proto.SetConfigRequest{
ProfileName: profName,
Username: username,
ManagementUrl: "https://api.netbird.io",
})
require.NoError(t, err, "an implicit :443 is the same management URL")
}
// The kill switch still has to do its job: a request that moves a setting is
// refused, and the profile keeps the value it had.
func TestSetConfig_ChangingASettingIsRefused(t *testing.T) {
s, ctx, profName, username, cfgPath := setupServerWithProfile(t)
s.updateSettingsDisabled = true
_, err := s.SetConfig(ctx, &proto.SetConfigRequest{
ProfileName: profName,
Username: username,
ManagementUrl: "https://mgmt.elsewhere.example:443",
})
require.Error(t, err, "moving the management URL is a settings change")
require.Equal(t, codes.FailedPrecondition, gstatus.Code(err), "want the update-settings refusal, got %v", err)
cfg, err := profilemanager.GetExistingConfig(cfgPath)
require.NoError(t, err)
require.Equal(t, storedManagementURL, cfg.ManagementURL.String(), "the refused request changed the config anyway")
}
// A field whose requested value differs from the stored one is a change even
// when the rest of the request restates the configuration.
func TestSetConfig_SingleDivergingFieldIsRefused(t *testing.T) {
s, ctx, profName, username, _ := setupServerWithProfile(t)
s.updateSettingsDisabled = true
rosenpass := true
_, err := s.SetConfig(ctx, &proto.SetConfigRequest{
ProfileName: profName,
Username: username,
ManagementUrl: storedManagementURL,
RosenpassEnabled: &rosenpass,
})
require.Error(t, err, "enabling Rosenpass is a settings change")
require.Equal(t, codes.FailedPrecondition, gstatus.Code(err), "want the update-settings refusal, got %v", err)
}
// With the switch off, the same diverging request goes through: the gate must
// not leak into a daemon that never enabled it.
func TestSetConfig_ChangeAllowedWhenTheSwitchIsOff(t *testing.T) {
s, ctx, profName, username, cfgPath := setupServerWithProfile(t)
_, err := s.SetConfig(ctx, &proto.SetConfigRequest{
ProfileName: profName,
Username: username,
ManagementUrl: "https://mgmt.elsewhere.example:443",
})
require.NoError(t, err)
cfg, err := profilemanager.GetExistingConfig(cfgPath)
require.NoError(t, err)
require.Equal(t, "https://mgmt.elsewhere.example:443", cfg.ManagementURL.String())
}
// Login carries the same config surface as SetConfig, so it is gated the same
// way: a login that would move a protected setting is refused before it can
// touch daemon state.
func TestLogin_ChangingTheManagementURLIsRefused(t *testing.T) {
s, _, profName, username, cfgPath := setupServerWithProfile(t)
s.updateSettingsDisabled = true
s.rootCtx = internal.CtxInitState(context.Background())
cancelled := false
s.actCancel = func() { cancelled = true }
_, err := s.Login(userCtx(), &proto.LoginRequest{
Username: &username,
ManagementUrl: "https://mgmt.elsewhere.example:443",
})
require.Error(t, err, "moving the management URL through Login is a settings change")
require.Equal(t, codes.FailedPrecondition, gstatus.Code(err), "want the update-settings refusal, got %v", err)
// "Refused before it can touch daemon state" is the contract, so check the
// state as well as the error.
cfg, err := profilemanager.GetExistingConfig(cfgPath)
require.NoError(t, err)
require.Equal(t, storedManagementURL, cfg.ManagementURL.String(), "the refused login moved the management URL")
require.False(t, cancelled, "the refused login cancelled the login already in progress")
active, err := s.profileManager.GetActiveProfileState()
require.NoError(t, err)
require.Equal(t, profilemanager.ID(profName), active.ID, "the refused login switched the active profile")
}
// seedProfileConfig writes a profile config carrying the given management URL
// and pre-shared key into a temp dir, and returns its path.
func seedProfileConfig(t *testing.T, managementURL, preSharedKey string) string {
t.Helper()
path := filepath.Join(t.TempDir(), "seeded.json")
_, err := profilemanager.UpdateOrCreateConfig(profilemanager.ConfigInput{
ConfigPath: path,
ManagementURL: managementURL,
PreSharedKey: &preSharedKey,
})
require.NoError(t, err, "seed profile config")
return path
}
// The decision procedure itself, over the fields a login actually persists.
// A login that restates the stored values must not be refused: that is what
// keeps a re-login, or a container restart carrying NB_MANAGEMENT_URL, working
// with the kill switch on.
func TestLoginGateDecision(t *testing.T) {
stored, err := profilemanager.GetExistingConfig(seedProfileConfig(t, storedManagementURL, "stored-key"))
require.NoError(t, err)
redacted := mdm.PreSharedKeyRedactedSentinel
empty := ""
sameKey := "stored-key"
otherKey := "other-key"
tests := []struct {
name string
msg *proto.LoginRequest
wantChanged bool
}{
{
name: "pure auth carries no config",
msg: &proto.LoginRequest{SetupKey: "ABC"},
wantChanged: false,
},
{
name: "stored management URL restated",
msg: &proto.LoginRequest{ManagementUrl: storedManagementURL},
wantChanged: false,
},
{
name: "stored management URL without its default port",
msg: &proto.LoginRequest{ManagementUrl: "https://api.netbird.io"},
wantChanged: false,
},
{
name: "different management URL",
msg: &proto.LoginRequest{ManagementUrl: "https://mgmt.elsewhere.example:443"},
wantChanged: true,
},
{
name: "stored pre-shared key restated",
msg: &proto.LoginRequest{OptionalPreSharedKey: &sameKey},
wantChanged: false,
},
{
name: "redacted pre-shared key echoed back",
msg: &proto.LoginRequest{OptionalPreSharedKey: &redacted},
wantChanged: false,
},
{
name: "empty pre-shared key is not a request to clear it",
msg: &proto.LoginRequest{OptionalPreSharedKey: &empty},
wantChanged: false,
},
{
name: "different pre-shared key",
msg: &proto.LoginRequest{OptionalPreSharedKey: &otherKey},
wantChanged: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
require.Equal(t, tt.wantChanged, configChangeRequested(stored, loginOverridesInput(tt.msg)))
})
}
}
// A profile with no config on disk yet is judged against the config the daemon
// would create for it, so a first login that asks for the defaults is not a
// change while one that asks for a different management URL is.
func TestGateDecisionWithoutStoredConfig(t *testing.T) {
require.False(t, configChangeRequested(nil, profilemanager.ConfigInput{}),
"a request carrying nothing cannot change anything")
require.False(t, configChangeRequested(nil, profilemanager.ConfigInput{ManagementURL: profilemanager.DefaultManagementURL}),
"asking for the default management URL is what the daemon would write anyway")
require.True(t, configChangeRequested(nil, profilemanager.ConfigInput{ManagementURL: "https://mgmt.elsewhere.example:443"}),
"asking for a non-default management URL is a change")
}
// A dry run that cannot be evaluated must fail closed, or a malformed field
// would open the gate.
func TestGateDecisionFailsClosedOnAnInvalidRequest(t *testing.T) {
require.True(t, configChangeRequested(nil, profilemanager.ConfigInput{ManagementURL: "not-a-url"}),
"an unevaluable request must count as a change")
}
// The gate reads the stored config to decide, and reading it must not write it:
// a refused request has to leave the profile file byte-for-byte as it was.
// A config file missing a field the config layer fills in (MTU, here) is what
// makes the normalization write fire.
func TestSetConfig_RefusedRequestLeavesTheConfigFileUntouched(t *testing.T) {
s, ctx, profName, username, cfgPath := setupServerWithProfile(t)
s.updateSettingsDisabled = true
require.NoError(t, os.WriteFile(cfgPath, []byte(`{"WgIface":"wt0"}`), 0o600))
before, err := os.ReadFile(cfgPath)
require.NoError(t, err)
_, err = s.SetConfig(ctx, &proto.SetConfigRequest{
ProfileName: profName,
Username: username,
ManagementUrl: "https://mgmt.elsewhere.example:443",
})
require.Error(t, err)
require.Equal(t, codes.FailedPrecondition, gstatus.Code(err), "want the update-settings refusal, got %v", err)
after, err := os.ReadFile(cfgPath)
require.NoError(t, err)
require.Equal(t, string(before), string(after), "the refused request rewrote the profile config")
}
// The container case that the string comparison still broke: the management URL
// supplied through the environment is the stored one, written with a trailing
// slash.
func TestSetConfig_ManagementURLSpellingsPassTheGate(t *testing.T) {
for _, spelling := range []string{
"https://api.netbird.io",
"https://api.netbird.io/",
"https://api.netbird.io:443/",
"https://API.netbird.io:443",
} {
t.Run(spelling, func(t *testing.T) {
s, ctx, profName, username, _ := setupServerWithProfile(t)
s.updateSettingsDisabled = true
_, err := s.SetConfig(ctx, &proto.SetConfigRequest{
ProfileName: profName,
Username: username,
ManagementUrl: spelling,
})
require.NoError(t, err, "%q is the stored management URL written differently", spelling)
})
}
}
// The RPC the whole fix hangs on. Login is retried by the CLI in a backoff
// loop, so a login that restates the stored configuration — which is what a
// container configured by environment sends on every start — must get past the
// gate, or the client never comes up at all.
//
// Past the gate the handler goes on to do real work this test does not stand
// up, so the assertion is only that the refusal did not happen.
func TestLogin_RestatingTheStoredConfigPassesTheGate(t *testing.T) {
s, _, _, username, _ := setupServerWithProfile(t)
s.updateSettingsDisabled = true
s.rootCtx = internal.CtxInitState(context.Background())
// Stand in for the management round trip the handler makes once the gate
// lets it through, so this test exercises the gate and not the network:
// without it the profile's management URL is dialed for real.
s.isLoginRequiredFn = func(context.Context) (bool, error) { return false, nil }
_, err := s.Login(userCtx(), &proto.LoginRequest{
Username: &username,
ManagementUrl: storedManagementURL,
})
if err != nil {
require.NotEqual(t, codes.FailedPrecondition, gstatus.Code(err),
"the gate refused a login that changes nothing: %v", err)
require.NotContains(t, err.Error(), "update settings are disabled",
"the gate refused a login that changes nothing: %v", err)
}
}
// The value-aware decision has the same synchronization problem as the
// privileged-change one: Login's first check runs outside guardedConfigMu, so
// the stored config it compared against can move before the write. A login that
// was a no-op when it was checked must not be written once it has become a
// change.
func TestLogin_ChangeThatAppearsMidRequestIsRefused(t *testing.T) {
s, _, _, username, _ := setupServerWithProfile(t)
s.updateSettingsDisabled = true
s.rootCtx = internal.CtxInitState(context.Background())
target := "moved-under-us"
targetPath := filepath.Join(profilemanager.DefaultConfigPathDir, target+".json")
_, err := profilemanager.UpdateOrCreateConfig(profilemanager.ConfigInput{
ConfigPath: targetPath,
ManagementURL: storedManagementURL,
})
require.NoError(t, err)
cancelled := false
s.actCancel = func() { cancelled = true }
// Stand in for a concurrent writer that repoints the profile between the two
// checks, which is the interleaving the lock has to make safe. The login
// restates the URL the profile held when it was checked, so the first check
// sees a no-op and lets it through.
afterLoginPreCheck = func() {
_, err := profilemanager.UpdateOrCreateConfig(profilemanager.ConfigInput{
ConfigPath: targetPath,
ManagementURL: "https://mgmt.elsewhere.example:443",
})
require.NoError(t, err)
}
t.Cleanup(func() { afterLoginPreCheck = nil })
_, err = s.Login(userCtx(), &proto.LoginRequest{
ProfileName: &target,
Username: &username,
ManagementUrl: storedManagementURL,
})
require.Error(t, err, "the login became a settings change before it was written")
require.Equal(t, codes.FailedPrecondition, gstatus.Code(err), "want the update-settings refusal, got %v", err)
require.False(t, cancelled, "the refused login cancelled the login already in progress")
stored, err := profilemanager.GetExistingConfig(targetPath)
require.NoError(t, err)
require.Equal(t, "https://mgmt.elsewhere.example:443", stored.ManagementURL.String(),
"the refused login wrote the management URL it was asked for")
}
// Logging out a profile that was already logged out must not fail: the logout
// clears the keys in place, so the second attempt finds a profile with no
// identity, which was never registered and has nothing to deregister.
func TestLogout_ProfileWithoutAnIdentityIsANoOp(t *testing.T) {
s, _, _, _, cfgPath := setupServerWithProfile(t)
loggedOut, err := profilemanager.GetExistingConfig(cfgPath)
require.NoError(t, err)
loggedOut.PrivateKey = ""
loggedOut.SSHKey = ""
require.NoError(t, profilemanager.WriteOutConfig(cfgPath, loggedOut))
stored, err := profilemanager.GetExistingConfig(cfgPath)
require.NoError(t, err)
require.NoError(t, s.sendLogoutRequestWithConfig(privilegedTestCtx(), stored),
"logging out an identity-less profile must not fail")
// And for an unprivileged caller too: the deregistration privilege gate
// guards the handover of a registered key, so with no key there is nothing
// to guard. An unset SSH setting is what arms that gate — sshServerEnabled
// reads an absent value as enabled — so this stands in for every legacy
// profile, where behind the gate the caller would be refused.
stored.ServerSSHAllowed = nil
require.NoError(t, s.sendLogoutRequestWithConfig(userCtx(), stored),
"an unprivileged caller could not log out a profile with nothing to deregister")
}