diff --git a/client/mobile/profile_manager.go b/client/mobile/profile_manager.go index 2aec6eb2c..d8633b96f 100644 --- a/client/mobile/profile_manager.go +++ b/client/mobile/profile_manager.go @@ -174,7 +174,10 @@ func (pm *ProfileManager) LogoutProfile(id string) error { return fmt.Errorf("profile %q does not exist", id) } - config, err := profilemanager.ReadOrGenerateConfig(configPath) + // The existing-file reader, not the generating one: the check above is not + // atomic with this read, so a profile removed in between would otherwise be + // resolved from the defaults here and recreated by the write below. + config, err := profilemanager.GetExistingConfig(configPath) if err != nil { return fmt.Errorf("read profile config: %w", err) } diff --git a/client/server/server.go b/client/server/server.go index 9d9bcce28..5807cff64 100644 --- a/client/server/server.go +++ b/client/server/server.go @@ -1577,6 +1577,16 @@ func (s *Server) sendLogoutRequestWithConfig(ctx context.Context, config *profil return err } + // A profile with no identity was never registered — a logout clears the + // keys in place, so logging the same profile out twice lands here — and + // there is nothing to deregister. Reads no longer mint a key on the way + // past, so this is where that case surfaces instead of dialing management + // with a key it has never seen. + if config.PrivateKey == "" { + log.Infof("profile carries no identity, nothing to deregister") + return nil + } + key, err := wgtypes.ParseKey(config.PrivateKey) if err != nil { return fmt.Errorf("parse private key: %w", err) diff --git a/client/server/update_settings_gate_test.go b/client/server/update_settings_gate_test.go index 667c963a3..e0ea8bcf9 100644 --- a/client/server/update_settings_gate_test.go +++ b/client/server/update_settings_gate_test.go @@ -360,3 +360,21 @@ func TestLogin_ChangeThatAppearsMidRequestIsRefused(t *testing.T) { 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") +}