diff --git a/client/internal/profilemanager/config.go b/client/internal/profilemanager/config.go index 3ae9ea852..87c926739 100644 --- a/client/internal/profilemanager/config.go +++ b/client/internal/profilemanager/config.go @@ -1443,17 +1443,15 @@ func ConfigToJSON(config *Config) (string, error) { // ConfigFromJSON deserializes a JSON string to a Config struct. // This is useful for restoring config from alternative storage mechanisms. // After unmarshaling, defaults are applied to ensure the config is fully -// initialized. The peer identity is not one of those defaults: a document -// carrying none is refused with ErrConfigWithoutIdentity. +// initialized. // -// Provisioning one here would be worse than refusing. Both callers connect -// with what they get back — the iOS SDK's Client.SetConfigFromJSON keeps it as -// the preloaded config Run() uses, and Auth.SetConfigFromJSON as the config it -// authenticates with — and neither can hand a generated key back to the store -// the document came from, since Client exports no config at all. The peer -// would connect under an identity nothing persists and re-register on every -// launch. A document with no identity means nobody has logged in yet, and -// that is what the caller has to be told. +// The peer identity is deliberately none of its business, in either direction. +// It does not generate one: a read cannot hand back keys that nothing will +// write down (see ReadOrGenerateConfig). Nor does it refuse a document that +// carries none, because a config legitimately has no identity between a logout +// and the next login — mobile logout clears both keys in place — and this is +// also the deserializer the iOS SDK copies a config through. Whoever goes on +// to connect is where an absent identity has to be answered. func ConfigFromJSON(jsonStr string) (*Config, error) { config := &Config{} err := json.Unmarshal([]byte(jsonStr), config) @@ -1467,12 +1465,5 @@ func ConfigFromJSON(jsonStr string) (*Config, error) { return nil, fmt.Errorf("failed to apply defaults to config: %w", err) } - // Both keys, because both are dead ends when missing: an empty WireGuard - // key fails the management login on its size, and an empty SSH key fails - // ssh.GeneratePublicKey in ConnectClient before the engine starts. - if config.PrivateKey == "" || config.SSHKey == "" { - return nil, ErrConfigWithoutIdentity - } - return config, nil } diff --git a/client/internal/profilemanager/config_json_test.go b/client/internal/profilemanager/config_json_test.go index 3000ee8a0..9a6d820c4 100644 --- a/client/internal/profilemanager/config_json_test.go +++ b/client/internal/profilemanager/config_json_test.go @@ -7,16 +7,16 @@ import ( "github.com/stretchr/testify/require" ) -// The serialized form is how the tvOS SDK stores a profile, and its callers -// connect with whatever comes back. A document with no identity used to be -// completed by apply() minting keys, which meant connecting as a peer nothing -// could persist; it is now refused, since the only honest answer to "restore -// this config" for a config that was never logged in is to say so. -func TestConfigFromJSONRefusesADocumentWithoutAnIdentity(t *testing.T) { +// The serialized form is how the tvOS SDK stores a profile and how the iOS SDK +// copies one in memory, so it must round-trip whatever a profile legitimately +// holds — including no identity at all, which is the state mobile logout leaves +// behind when it clears both keys in place. Refusing that document here broke +// logout, profile switching and the login that follows them. +func TestConfigFromJSONRoundTripsALoggedOutProfile(t *testing.T) { path := filepath.Join(t.TempDir(), "exported.json") stored, err := UpdateOrCreateConfig(ConfigInput{ConfigPath: path, ManagementURL: DefaultManagementURL}) require.NoError(t, err) - require.NotEmpty(t, stored.PrivateKey, "a provisioned config is the fixture this test needs") + require.NotEmpty(t, stored.PrivateKey, "a provisioned config is the fixture this test starts from") require.NotEmpty(t, stored.SSHKey) exported, err := ConfigToJSON(stored) @@ -27,23 +27,18 @@ func TestConfigFromJSONRefusesADocumentWithoutAnIdentity(t *testing.T) { require.Equal(t, stored.PrivateKey, restored.PrivateKey, "the restored peer is not the stored one") require.Equal(t, stored.SSHKey, restored.SSHKey) - for _, missing := range []struct { - name string - strip func(*Config) - }{ - {"no WireGuard key", func(c *Config) { c.PrivateKey = "" }}, - {"no SSH key", func(c *Config) { c.SSHKey = "" }}, - {"no keys at all", func(c *Config) { c.PrivateKey = ""; c.SSHKey = "" }}, - } { - t.Run(missing.name, func(t *testing.T) { - incomplete := stored.clone() - missing.strip(incomplete) + // What mobile logout leaves on disk. + loggedOut := stored.clone() + loggedOut.PrivateKey = "" + loggedOut.SSHKey = "" - document, err := ConfigToJSON(incomplete) - require.NoError(t, err) + document, err := ConfigToJSON(loggedOut) + require.NoError(t, err) - _, err = ConfigFromJSON(document) - require.ErrorIs(t, err, ErrConfigWithoutIdentity) - }) - } + reloaded, err := ConfigFromJSON(document) + require.NoError(t, err, "a logged-out profile must still load") + require.Empty(t, reloaded.PrivateKey, "loading must not mint a key nothing will write down") + require.Empty(t, reloaded.SSHKey) + require.Equal(t, stored.ManagementURL.String(), reloaded.ManagementURL.String(), + "the rest of the profile survives the logout") } diff --git a/client/internal/profilemanager/error.go b/client/internal/profilemanager/error.go index b5a1ad7e0..d83fe5c1c 100644 --- a/client/internal/profilemanager/error.go +++ b/client/internal/profilemanager/error.go @@ -6,9 +6,4 @@ var ( ErrProfileNotFound = errors.New("profile not found") ErrProfileAlreadyExists = errors.New("profile already exists") ErrNoActiveProfile = errors.New("no active profile set") - - // ErrConfigWithoutIdentity is returned for a serialized config that carries - // no WireGuard or SSH key. See ConfigFromJSON for why it is refused rather - // than provisioned. - ErrConfigWithoutIdentity = errors.New("config carries no peer identity: log in to provision one before loading a stored config") ) diff --git a/client/ios/NetBirdSDK/client.go b/client/ios/NetBirdSDK/client.go index c24366e7f..a6315a4ab 100644 --- a/client/ios/NetBirdSDK/client.go +++ b/client/ios/NetBirdSDK/client.go @@ -130,13 +130,12 @@ func NewClient(cfgFile, stateFile, cacheDir, logFilePath, deviceName string, osV // SetConfigFromJSON stores the JSON config that later loads resolve instead of the config file (tvOS). func (c *Client) SetConfigFromJSON(jsonStr string) error { - // Parsed only to reject a bad document early; the JSON itself is what is - // stored, and every load re-parses it. Not only a parse error any more: a - // document with no peer identity is refused too, because Run() would - // otherwise connect as a peer whose key this SDK has no way to hand back - // to the caller's store. + // Parsed only to reject an unreadable document early; the JSON itself is + // what is stored, and every load re-parses it. A document carrying no peer + // identity is readable and accepted: that is a logged-out profile, and the + // login that follows provisions the keys. if _, err := profilemanager.ConfigFromJSON(jsonStr); err != nil { - log.Errorf("SetConfigFromJSON: failed to load config JSON: %v", err) + log.Errorf("SetConfigFromJSON: failed to parse config JSON: %v", err) return err } c.preloadedConfigJSON.Store(&jsonStr)