Merge remote-tracking branch 'origin/main' into fix_debug_upload_url_from_mgmt

# Conflicts:
#	client/proto/daemon.pb.go
#	management/internals/controllers/network_map/controller/controller.go
#	management/internals/controllers/network_map/nmaptest/runner.go
#	management/internals/shared/grpc/server.go
#	management/server/account.go
#	shared/management/proto/management.pb.go
#	upload-server/server/server.go
This commit is contained in:
riccardom
2026-10-09 11:13:57 +02:00
518 changed files with 30626 additions and 12765 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
}
+211
View File
@@ -0,0 +1,211 @@
package server
import (
"context"
"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/proto"
)
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 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())
-86
View File
@@ -180,92 +180,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.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.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")
})
}
+323 -121
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
@@ -172,6 +187,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.
@@ -492,16 +515,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
@@ -513,19 +547,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)
@@ -641,37 +666,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
}
@@ -741,7 +774,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
@@ -757,23 +790,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
}
@@ -783,11 +824,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)
@@ -799,32 +842,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
@@ -897,9 +982,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")
}
@@ -917,31 +1009,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
@@ -954,7 +1051,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):
@@ -970,9 +1069,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)
@@ -1174,6 +1296,10 @@ func (s *Server) storedLoginConfig(activeProf *profilemanager.ActiveProfileState
// storedConfigAtPath reads a profile config file, yielding nil when it does not
// exist yet.
//
// 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) {
@@ -1182,7 +1308,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)
}
@@ -1278,6 +1404,22 @@ func (s *Server) SwitchProfile(callerCtx context.Context, msg *proto.SwitchProfi
s.jwtCache.clear()
// 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()
if msg != nil && msg.ProfileName != nil {
s.publishProfileListChanged(*msg.ProfileName)
}
@@ -1485,8 +1627,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)
@@ -1497,15 +1647,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
@@ -1560,7 +1733,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)
}
@@ -1579,6 +1752,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
}
@@ -1877,7 +2063,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)
}
@@ -1887,12 +2073,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,
@@ -1992,7 +2181,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)
}
@@ -2196,7 +2385,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)
@@ -2659,8 +2848,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()
@@ -2691,6 +2878,15 @@ func (s *Server) authorizeAndPrepareLogin(callerCtx context.Context, msg *proto.
return nil, nil, 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, gstatus.Errorf(codes.FailedPrecondition, errUpdateSettingsDisabled)
}
s.mutex.Lock()
if s.actCancel != nil {
s.actCancel()
@@ -2717,18 +2913,28 @@ func (s *Server) authorizeAndPrepareLogin(callerCtx context.Context, msg *proto.
return nil, nil, fmt.Errorf("active profile state: %w", err)
}
if err := persistLoginOverrides(activeProf, msg.ManagementUrl, msg.OptionalPreSharedKey); err != nil {
if err := persistLoginOverrides(activeProf, msg); err != nil {
return nil, nil, fmt.Errorf("persist login overrides: %w", err)
}
// 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, err
}
return ctx, activeProf, 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
}
@@ -2737,11 +2943,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(),
}
}
+2 -3
View File
@@ -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
@@ -290,7 +290,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
@@ -125,7 +125,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
@@ -331,21 +331,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")
}