Fix on demand & auth pop-up#85
Merged
Merged
Conversation
📝 WalkthroughWalkthroughAdds periodic login-required detection to the iOS polling loop, preserves UI state during VPN connecting/disconnecting transitions, guards against overlapping extension-status requests, updates Network Extension login/start behavior to signal login earlier, and changes the authentication alert to offer "Login" or "Later". Changes
Sequence Diagram(s)sequenceDiagram
participant App as MainViewModel
participant Poll as PollingLoop
participant Adapter as NetworkExtensionAdapter
participant Ext as NetworkExtension
participant UI as MainView
Poll->>App: startPollingDetails()
activate Poll
Note over Poll: periodic cycle
Poll->>App: checkExtensionState()
App->>Adapter: getExtensionStatus()
Adapter->>Ext: query status
Ext-->>Adapter: status
Adapter-->>App: status completion
App->>App: updateVPNDisplayState()
Poll->>App: checkLoginRequiredFlag()
App->>Adapter: needsLogin()
Adapter->>Ext: query needsLogin()
Ext-->>Adapter: login required
Adapter-->>App: notify login required
App->>App: clear connectPressed, update UI
App->>Adapter: setOnDemandEnabled(false)
App->>UI: schedule local notification / show alert
UI->>App: user taps "Login"
App->>Adapter: loginIfRequired()
Adapter->>Ext: performLogin()
Ext-->>Adapter: login result
Adapter->>Ext: startVPNConnection() (if showBrowser == false)
deactivate Poll
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
NetBird/Source/App/ViewModels/MainViewModel.swift (1)
491-505: Concurrency guard logic is correct, but consider a timeout safeguard.The guard effectively prevents overlapping
getExtensionStatuscalls and out-of-order completions. Since the class is@MainActor, the flag access is thread-safe.One edge case: if
getExtensionStatusnever invokes its completion handler (e.g., due to a bug or hang inloadAllFromPreferences),isCheckingExtensionStateremainstrueindefinitely, blocking all future status checks. Consider adding a timeout reset:🛡️ Optional: Add timeout safeguard
func checkExtensionState() { guard !isCheckingExtensionState else { return } isCheckingExtensionState = true + + // Safety timeout in case completion is never called + DispatchQueue.main.asyncAfter(deadline: .now() + 5) { [weak self] in + self?.isCheckingExtensionState = false + } + networkExtensionAdapter.getExtensionStatus { status in DispatchQueue.main.async { self.isCheckingExtensionState = false self.applyExtensionStatus(status) } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@NetBird/Source/App/ViewModels/MainViewModel.swift` around lines 491 - 505, Add a timeout safeguard to avoid permanently blocking future checks if networkExtensionAdapter.getExtensionStatus never calls its completion: in checkExtensionState (which uses isCheckingExtensionState) start a fallback timer (e.g. DispatchQueue.main.asyncAfter) when you set isCheckingExtensionState = true that, after a short interval, resets isCheckingExtensionState = false and optionally calls applyExtensionStatus(.unknown) or triggers another safe retry; cancel or ignore the fallback when the real completion runs so only one reset occurs; reference checkExtensionState, isCheckingExtensionState, networkExtensionAdapter.getExtensionStatus, and applyExtensionStatus when implementing this.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@NetBird/Source/App/ViewModels/MainViewModel.swift`:
- Around line 491-505: Add a timeout safeguard to avoid permanently blocking
future checks if networkExtensionAdapter.getExtensionStatus never calls its
completion: in checkExtensionState (which uses isCheckingExtensionState) start a
fallback timer (e.g. DispatchQueue.main.asyncAfter) when you set
isCheckingExtensionState = true that, after a short interval, resets
isCheckingExtensionState = false and optionally calls
applyExtensionStatus(.unknown) or triggers another safe retry; cancel or ignore
the fallback when the real completion runs so only one reset occurs; reference
checkExtensionState, isCheckingExtensionState,
networkExtensionAdapter.getExtensionStatus, and applyExtensionStatus when
implementing this.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0bfa629b-0843-4c15-976a-607112dc19be
📒 Files selected for processing (1)
NetBird/Source/App/ViewModels/MainViewModel.swift
pappz
approved these changes
Apr 21, 2026
This was referenced Apr 21, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Other
PR Description: This PR fixes an issue with On-Demand (Always On) VPN where users could not re-authenticate after session (peer) expiration.
Problem: When On-Demand was enabled and the session expired:
The app did not show the login screen
User was unable to re-authenticate
Disabling On-Demand immediately resolved the issue (login flow worked as expected)
Steps to Reproduce (Before Fix)
(or install the app fresh and attempt initial login)
Result:
App attempts to connect
Login screen is not shown
User cannot re-authenticate
What’s Fixed
• Added reliable detection of login-required state during background polling
• Improved VPN state handling to preserve correct UI behavior during transitions
• Prevented overlapping extension status checks (avoids stale UI state)
How to Test (After Fix)
User is prompted with a re-login pop-up
Authentication succeeds
VPN reconnects correctly