Skip to content

Conversation

@mlsmaycon
Copy link
Contributor

@mlsmaycon mlsmaycon commented Jan 26, 2026

ChromeOS runs Android apps in a container with separate network namespace,
preventing the browser from reaching the localhost callback server used by
PKCE flow. This causes "service not available" errors after authentication.

Use Device Code Flow on ChromeOS (like Android TV) which uses polling
instead of localhost callback. On ChromeOS, also auto-open the browser
for a similar UX to PKCE while showing QR dialog as fallback.

Summary by CodeRabbit

  • New Features

    • Introduced device code flow authentication for ChromeOS and Android TV.
    • Added QR code sign-in with optional browser auto-launch on compatible devices for streamlined login.
    • Enhanced platform detection to better identify ChromeOS devices.
  • Bug Fixes

    • Minor logging improvements for clearer diagnostics during device code flow.

✏️ Tip: You can customize this high-level summary in your review settings.

  ChromeOS runs Android apps in a container with separate network namespace,
  preventing the browser from reaching the localhost callback server used by
  PKCE flow. This causes "service not available" errors after authentication.

  Use Device Code Flow on ChromeOS (like Android TV) which uses polling
  instead of localhost callback. On ChromeOS, also auto-open the browser
  for a similar UX to PKCE while showing QR dialog as fallback.
@coderabbitai
Copy link

coderabbitai bot commented Jan 26, 2026

📝 Walkthrough

Walkthrough

Adds device code flow detection and support: new PlatformUtils methods detect ChromeOS and require device-code flow for ChromeOS/Android TV; MainActivity gains a useDeviceCodeFlow flag and switches authentication, URL-opening, engine run/bind, and UI logic to use device code flow where appropriate.

Changes

Cohort / File(s) Change Summary
Platform detection
app/src/main/java/io/netbird/client/PlatformUtils.java
Added isChromeOS(Context) (checks PackageManager features) and requiresDeviceCodeFlow(Context) (returns true for Android TV or ChromeOS).
Main activity: device-code integration
app/src/main/java/io/netbird/client/MainActivity.java
Added private boolean useDeviceCodeFlow = false; populated by PlatformUtils.requiresDeviceCodeFlow(...). Replaced multiple isRunningOnTV checks with useDeviceCodeFlow to control URL opener initialization, browser launch for device-code flow (non-TV), auth success logging, engine bind/run calls, drawer closing, and VPN-permission engine start. Added error handling/logging around browser launch and ChromeOS-specific logs.
Submodule pointer
.../netbird
Updated submodule reference commit pointer (no code changes).

Sequence Diagram

sequenceDiagram
    participant MA as MainActivity
    participant PU as PlatformUtils
    participant PM as PackageManager
    participant Auth as AuthHandler
    participant QR as QRDialog
    participant Browser as Browser

    MA->>PU: requiresDeviceCodeFlow(context)
    PU->>PU: isAndroidTV(context)?
    alt Android TV
        PU-->>MA: true
    else
        PU->>PM: hasSystemFeature("org.chromium.arc" / "org.chromium.arc.device_management")
        PM-->>PU: feature result
        alt ChromeOS detected
            PU-->>MA: true
        else
            PU-->>MA: false
        end
    end

    alt useDeviceCodeFlow == true
        MA->>Auth: initiate device-code auth flow
        Auth-->>MA: provide device-code URL
        MA->>QR: show QR dialog with code/URL
        alt device is not TV
            MA->>Browser: try to launch URL in browser
            Browser-->>MA: success / error
        end
    else
        MA->>Auth: start standard OAuth flow (URL opener)
    end

    Auth-->>MA: onLoginSuccess
    MA->>MA: run/bind engine with useDeviceCodeFlow flag
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • doromaraujo

Poem

🐰 I hopped through code on ChromeOS and TV,
QR trails led the logins to me,
Browsers launched with a tentative cheer,
Engines wake up when the flags appear,
Hop, scan, connect — the rabbit's near! 🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fix SSO login on ChromeOS by using Device Code Flow' accurately summarizes the main change: implementing Device Code Flow authentication for ChromeOS to resolve SSO login failures caused by network namespace isolation.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
app/src/main/java/io/netbird/client/MainActivity.java (1)

185-229: Guard DialogFragment dismissal against state loss after browser launch.

The onLoginSuccess() callback runs on a background thread from the OAuth library and attempts to dismiss qrCodeDialog. When the browser is launched via startActivity, the Activity enters onSaveInstanceState, but the callback may fire after this state is saved (when login completes). Calling dismiss() then throws IllegalStateException.

Wrap the dismiss call with runOnUiThread() to ensure UI thread execution, check getSupportFragmentManager().isStateSaved(), and use dismissAllowingStateLoss() when state has been saved.

Proposed fix
                 `@Override`
                 public void onLoginSuccess() {
                     Log.d(LOGTAG, "onLoginSuccess fired for device code flow.");
-                    if (qrCodeDialog != null && qrCodeDialog.isVisible()) {
-                        qrCodeDialog.dismiss();
-                        qrCodeDialog = null;
-                    }
+                    if (qrCodeDialog != null && qrCodeDialog.isVisible()) {
+                        runOnUiThread(() -> {
+                            if (getSupportFragmentManager().isStateSaved()) {
+                                qrCodeDialog.dismissAllowingStateLoss();
+                            } else {
+                                qrCodeDialog.dismiss();
+                            }
+                            qrCodeDialog = null;
+                        });
+                    }
                 }

@mlsmaycon mlsmaycon merged commit 98a2ee7 into main Jan 26, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants