From 7546e7751cecffcf98f1b55e1feed88bd643da17 Mon Sep 17 00:00:00 2001 From: camiloariza <14282973+camiloariza@users.noreply.github.com> Date: Mon, 3 Aug 2026 13:42:15 -0300 Subject: [PATCH] [client, android] Reuse the persisted configuration when enrolling (#7022) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Describe your changes NewAuth builds a fresh in-memory configuration on every call, which means a new WireGuard key each time. The peer registers under that key and the key is written out, so any peer registered by an earlier call is orphaned on the server — a client that enrols twice leaves two entries and owns neither. It also breaks the enrol-then-run sequence. `RunWithoutLogin` reloads the configuration from disk through `UpdateOrCreateConfig`, so the identity that registered is not necessarily the identity that runs, and the management stream rejects it: ``` failed to login to Management Service: rpc error: code = PermissionDenied desc = no peer auth method provided, please use a setup key or interactive SSO login ``` followed by a panic in `ConnectClient.run`. ### How it was found Embedding the Android client in an application that enrols with a setup key and then runs. Eight orphaned peers accumulated on a self-hosted management server before the cause was clear, because every restart registered a new one. ### The change `NewAuth` passes `ConfigPath` and uses `UpdateOrCreateConfig`, so an existing configuration is reused and one is only created when absent. A caller wanting a fresh identity can delete the file — which is what "forget this account" already does. ### Test `TestNewAuth_ReusesPersistedIdentity` fails on the current code: ``` --- FAIL: TestNewAuth_ReusesPersistedIdentity (0.00s) login_test.go:33: private key changed between calls: a second enrolment would orphan the peer registered by the first ``` and passes with the fix. `TestNewAuth_CreatesConfigWhenAbsent` covers the first-enrolment path being unchanged. Both run in `client/android` on Linux. Per CONTRIBUTING, opening directly as a bug fix rather than raising an issue first. ## Issue ticket number and link [NET-1465](https://linear.app/netbird/issue/NET-1465/agent-network-rest-api-settings-defaults-bootstrap-via-put-provider) ## Stack ### Checklist - [x] Is it a bug fix - [ ] Is a typo/documentation fix - [ ] Is a feature enhancement - [ ] It is a refactor - [ ] Created tests that fail without the change (if possible) - [ ] I ran and tested this change locally — I did not rely on CI to find out whether it works - [ ] This PR has a single purpose (not a fix + refactor + feature in one) - [ ] This change is a trivial fix, **OR** it links an issue the NetBird team agreed on beforehand. Changes to the public API, gRPC protocols, functionality behavior, CLI / service flags, or new features always need that agreement first. See [CONTRIBUTING.md](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTING.md#ticket-first-pr-second). > By submitting this pull request, you confirm that you have read and agree to the terms of the [Contributor License Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md). ## Documentation Select exactly one: - [ ] I added/updated documentation for this change - [x] Documentation is **not needed** for this change (explain why): the API reference is generated from the OpenAPI spec, which this PR updates in-repo. ### Docs PR URL (required if "docs added" is checked) Paste the PR link from https://github.com/netbirdio/docs here: https://github.com/netbirdio/docs/pull/__ Co-authored-by: Zoltan Papp --- client/android/login.go | 10 ++++++- client/android/login_test.go | 51 ++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 client/android/login_test.go diff --git a/client/android/login.go b/client/android/login.go index a9422cdbf..32ce28739 100644 --- a/client/android/login.go +++ b/client/android/login.go @@ -36,12 +36,20 @@ type Auth struct { } // NewAuth instantiate Auth struct and validate the management URL +// +// The configuration at cfgPath is reused when one is already there, and only created when it is +// not. Building a fresh in-memory config unconditionally gives the client a new WireGuard key on +// every call: the peer registers under that key, the key is written out, and any peer registered by +// an earlier call is orphaned on the server. It also breaks a client that enrols and then runs from +// the persisted config, because the identity it registered is not the one it runs with — the +// management stream rejects it with "no peer auth method provided". func NewAuth(cfgPath string, mgmURL string) (*Auth, error) { inputCfg := profilemanager.ConfigInput{ + ConfigPath: cfgPath, ManagementURL: mgmURL, } - cfg, err := profilemanager.CreateInMemoryConfig(inputCfg) + cfg, err := profilemanager.UpdateOrCreateConfig(inputCfg) if err != nil { return nil, err } diff --git a/client/android/login_test.go b/client/android/login_test.go new file mode 100644 index 000000000..b04790f6b --- /dev/null +++ b/client/android/login_test.go @@ -0,0 +1,51 @@ +package android + +import ( + "path/filepath" + "testing" +) + +// NewAuth must reuse the configuration already at cfgPath rather than building a fresh one. +// +// Creating a new in-memory config on every call gives the client a new WireGuard private key each +// time. The peer registers under that key and the key is written out, so a peer registered by an +// earlier call is orphaned on the server — a client that enrols twice leaves two entries and owns +// neither. It also breaks enrol-then-run: RunWithoutLogin reloads the configuration from disk, so +// the identity that registered is not the identity that runs, and the management stream rejects it +// with "no peer auth method provided, please use a setup key or interactive SSO login". +func TestNewAuth_ReusesPersistedIdentity(t *testing.T) { + cfgPath := filepath.Join(t.TempDir(), "config.json") + + first, err := NewAuth(cfgPath, "https://api.example.com:443") + if err != nil { + t.Fatalf("first NewAuth: %v", err) + } + if first.config.PrivateKey == "" { + t.Fatal("first NewAuth produced no private key") + } + + second, err := NewAuth(cfgPath, "https://api.example.com:443") + if err != nil { + t.Fatalf("second NewAuth: %v", err) + } + + if second.config.PrivateKey != first.config.PrivateKey { + t.Errorf("private key changed between calls: a second enrolment would orphan the peer registered by the first") + } +} + +// A missing configuration is still created, so a first enrolment works unchanged. +func TestNewAuth_CreatesConfigWhenAbsent(t *testing.T) { + cfgPath := filepath.Join(t.TempDir(), "config.json") + + auth, err := NewAuth(cfgPath, "https://api.example.com:443") + if err != nil { + t.Fatalf("NewAuth: %v", err) + } + if auth.config == nil || auth.config.PrivateKey == "" { + t.Fatal("NewAuth did not create a usable configuration") + } + if auth.cfgPath != cfgPath { + t.Errorf("cfgPath = %q, want %q", auth.cfgPath, cfgPath) + } +}