mobile: keep the PeerID when unenrolling - #429
Conversation
Unenroll deleted the whole data directory, taking the libp2p private key with it, so the device came back as a different node on every re-enrollment. Add a UnenrollNode FFI export that calls the store's existing ResetMeshIdentity -- the primitive behind `sam-node reset` -- which clears the Biscuit, refresh token, OIDC config, trusted keys and control-plane details while keeping node_private_key, so the PeerID survives. The dialog now offers both paths: Unenroll keeps the PeerID, Reset device identity still deletes the directory for a genuine first-enrollment state.
There was a problem hiding this comment.
Code Review
This pull request introduces an unenrollment feature allowing users to either unenroll the node (preserving the PeerID) or reset the device identity completely. Feedback on these changes highlights two key issues: first, in main.dart, the unenrollment process should abort if the node fails to stop to prevent potential database corruption; second, in ffi.go, a regression unit test is required for UnenrollNode to comply with Rule 3 of the Repository Style Guide.
| if (_running) { | ||
| _stop(); | ||
| } |
There was a problem hiding this comment.
If _stop() fails to stop the node (e.g., due to an error returned by _samLib.stop()), _running remains true. Proceeding to delete the data directory or calling _isolatedUnenroll while the node is still running can lead to database corruption, file lock errors, or unexpected crashes. We should abort the unenrollment process if the node fails to stop.
if (_running) {
_stop();
if (_running) {
return;
}
}There was a problem hiding this comment.
Checked this against the Go side, and the failure mode doesn't occur.
_stop() calls StopNode -> stopNodeInternal (mobile/sam-node-ffi/ffi/ffi.go:283), which releases the store and clears the node handles unconditionally. The error it returns comes from activeNode.Teardown() only, and it is returned after both have already been released:
var err error
if activeNode != nil {
err = activeNode.Teardown()
activeNode = nil
}
if activeStore != nil {
_ = activeStore.Close()
activeStore = nil
}
return errSo when _stop() reports a failure, the bbolt lock is already released and there is no live writer left. Both paths are safe: the recursive delete has nothing to corrupt, and UnenrollNode opens the store cleanly.
UnenrollNode also re-checks activeNode != nil || unauthSrv != nil before touching the store, which is what covers the genuine still-running case — including the one Dart cannot see, where the unauthenticated enrollment sidecar is up.
The suggested guard would additionally brick the feature. Nothing clears _running except a successful _stop(), so after a single Teardown error if (_running) return; makes unenroll impossible until the app is restarted, with the node actually down and the store actually closed.
Unenrolling in the mobile app deleted the whole data directory, which took the libp2p private key with it. Because an Ed25519 PeerID is the identity multihash of the public key, losing that key means the device rejoins the mesh as a different node every single time.
Store.ResetMeshIdentityalready does the right thing and is whatsam-node resethas always used: it clears the Biscuit, its expiration, the refresh token, the OIDC config, the trusted keys and the control-plane public key / URL / router addresses, while deliberately keepingnode_private_key. The only thing missing was a way for the app to reach it.Changes
UnenrollNode(dataDir)in the FFI layer: opens the store, callsResetMeshIdentity, closes. It keeps the same "stop the node first" guardReEnrollNodeuses, so a running node produces a clear error instead of a five-second wait on the bbolt file lock.sam_ffi.dart, run throughIsolate.runlike the other enrollment calls.The full wipe stays in Dart rather than gaining a second FFI export: on Android
sam_datais owned entirely by the app, so the recursive delete is safe, and a mode flag that selects between "keep the key" and "destroy the key" is the kind of argument that gets passed wrong once.Labels and the attenuation file survive an unenroll, so re-enrolling is one tap; the full reset takes them along with everything else.
Notes
Unenroll stays local, matching
sam-node reset- the control plane is not notified. A device asking to be forgotten is not a request the mesh can trust; the ban list remains the operator-side answer for a device that should not come back.Testing
go build ./...andgo vet ./mobile/...are clean, golangci-lint reports 0 issues, andinternal/nodeplusmobile/sam-node-ffi/ffitests pass. The Flutter side has not been analysed or run - no Flutter toolchain on the machine this was written on.