Skip to content

fix(wifi): use OWE token for Enhanced Open security mode (#1073)#1142

Merged
AustinChangLinksys merged 3 commits into
dev-2.6.0from
fix/wifi-owe-security-mode-1073
Jul 16, 2026
Merged

fix(wifi): use OWE token for Enhanced Open security mode (#1073)#1142
AustinChangLinksys merged 3 commits into
dev-2.6.0from
fix/wifi-owe-security-mode-1073

Conversation

@HankYuLinksys

Copy link
Copy Markdown
Collaborator

Summary

Fixes the app-side half of #1073.

Issue #1073 — In WiFi Settings, selecting the Enhanced Open (OWE) security mode and saving reports no error, but the mode silently reverts to its previous value. The user believes the change was saved when it was not.

Investigation (log analysis + on-device probing against Pinnacle 2.0, FW 1.2.2.26062820) showed the failure spans two layers: an app-side token bug and a firmware commit bug. This PR fixes the app side.

Root cause — two layers

App side (fixed in this PR)

The codebase used the string 'Enhanced-Open' for the Enhanced Open mode, but firmware only accepts the TR-181 token OWE and rejects 'Enhanced-Open' with "Out of range or invalid enumeration" (confirmed on device). This produced two latent bugs:

  1. 6 GHz open mode sent an invalid token. The 6 GHz override (_securityModeFor6GHz) sent 'Enhanced-Open', which firmware rejects outright.
  2. OWE networks were never recognized as open. isOpenSecurity / isPasswordRequired compared against 'Enhanced-Open', so a real OWE network was treated as non-open — the UI spuriously required a password and disabled Save.

Firmware side (NOT in this PR — still open)

Even with the correct OWE token, firmware accepts the SET with success: true (echoing OWE) but does not commit it — the post-save readback reverts to the previous mode. On-device probing confirmed:

  • SET {ModeEnabled: OWE}success: true, readback reverts to the old mode.
  • EncryptionMode is read-only, so the client cannot force it; no client-side field combination makes OWE stick.

This is a firmware bug and cannot be fixed from the app.

What this PR changes

Replaces all three 'Enhanced-Open' occurrences with 'OWE':

  • WifiNetworkUIModel.isOpenSecurity
  • WifiQuickSetupSettings.isPasswordRequired
  • _securityModeFor6GHz (6 GHz override + doc comment)

Tests:

  • Fixed a test fixture that asserted the old invalid token.
  • Added an isOpenSecurity group (None / empty / OWE / WPA variants).
  • Added a saveQuickSetup 6 GHz override group asserting the exact ModeEnabled value sent to firmware, guarding against a regression to 'Enhanced-Open'.

Impact

After this PR the app does everything correctly on its side:

  • Sends the valid OWE token on all bands.
  • Correctly treats OWE as an open (no-password) mode.

However, #1073 will remain reproducible until the firmware commit bug is fixed — the app now sends a valid OWE, but firmware still silently drops it. The remaining work is on the firmware team.

Verification

  • flutter test test/page/wifi_settings/ — all pass (175 tests).
  • Reverse-check confirmed the new OWE test fails if the token is reverted to 'Enhanced-Open'.
  • flutter analyze — no new issues.

The codebase used the string 'Enhanced-Open' for the Enhanced Open
(OWE) security mode, but firmware only accepts the TR-181 token 'OWE'
and rejects 'Enhanced-Open' as an invalid enumeration (verified on
device). This produced two latent app-side bugs, independent of the
firmware issue where OWE is accepted with success:true yet not
committed:

- 6 GHz open mode sent 'Enhanced-Open', which firmware rejects with
  "Out of range or invalid enumeration".
- OWE networks were never recognized as open, so the UI spuriously
  required a password (WifiQuickSetupSettings.isPasswordRequired) and
  blocked save.

Replace all three 'Enhanced-Open' occurrences with 'OWE':
- WifiNetworkUIModel.isOpenSecurity
- WifiQuickSetupSettings.isPasswordRequired
- _securityModeFor6GHz (6 GHz override + comment)

Fix the test fixture that asserted the old invalid token and add an
isOpenSecurity test group covering None / empty / OWE / WPA variants.
_securityModeFor6GHz had no direct test. Add a saveQuickSetup group
asserting the exact ModeEnabled value sent to firmware:
- 6 GHz + OWE / None (open) → 'OWE'
- 6 GHz + WPA3-Personal → 'WPA3-Personal'
- 2.4/5 GHz → selected mode written verbatim

Guards against regressing to the invalid 'Enhanced-Open' token.
@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@AustinChangLinksys AustinChangLinksys left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 Automated Review — Round 1 · f2af7bb..19bc7f0 (full)

Verdict: 💬 COMMENT — Critical issue found: saveAdvanced bypasses 6 GHz security override, allowing invalid security tokens to be written to firmware on 6 GHz networks.

Conf. Where Issue (one-liner)
🔴 🟢High usp_wifi_settings_service.dart:433–436 [both reviewers] saveAdvanced passes curr.securityMode raw to firmware — bypasses _securityModeFor6GHz() used by saveQuickSetup
⚠️ 🟢High wifi_ui.dart:108 [both reviewers] wifiDisplayValue has no case for 'OWE' — raw "OWE" shown in security mode display on WiFi status card
⚠️ 🟢High wifi_network_ui_model.dart:105 vs wifi_settings_settings.dart:55 [both reviewers] isOpenSecurity getter has zero callers in lib/; wifi_settings_settings.dart:55 re-implements the same open-mode check inline — dead code + DRY violation
💡 🟢High usp_wifi_settings_service_test.dart New 6 GHz tests cover saveQuickSetup path only — no test exercises saveAdvanced + 6 GHz band combination

Confidence: 🟢High = code-verified · 🟡Med = located + reasoned, not fully confirmed · ⚪Low = speculative, please double-check.
Items marked [both reviewers] were independently flagged by two agents → higher confidence.

🔴 Critical Details

C-1 — [both reviewers] saveAdvanced bypasses 6 GHz security mode override (Confidence: 🟢High)

Location: lib/page/wifi_settings/services/usp_wifi_settings_service.dart:433–436 (HEAD 19bc7f0)

Evidence chain — saveQuickSetup (correct path) vs saveAdvanced (broken path):

// saveQuickSetup — lines 311–315 — CORRECT: calls _securityModeFor6GHz
final band = bandByApPath[p] ?? '';
final securityMode = _securityModeFor6GHz(
  band: band,
  selectedMode: pending.securityMode,
);
// saveAdvanced — lines 433–436 — BUG: uses curr.securityMode verbatim, no override
securityModeEnabled:
    securityChanged && curr.securityMode.isNotEmpty
        ? curr.securityMode   // ← raw value, no _securityModeFor6GHz call
        : null,
// _securityModeFor6GHz — lines 635–641 — the guard that saveAdvanced skips
String _securityModeFor6GHz({required String band, required String selectedMode}) {
  if (!band.contains('6')) return selectedMode;
  const openModes = {'None', 'OWE', ''};
  return openModes.contains(selectedMode) ? 'OWE' : 'WPA3-Personal';
}

Why this is a bug: saveQuickSetup correctly calls _securityModeFor6GHz before writing Security.ModeEnabled to firmware, ensuring that on 6 GHz bands, only 'OWE' (open) or 'WPA3-Personal' are sent. saveAdvanced writes curr.securityMode verbatim — if a user in Advanced mode selects e.g. 'WPA2-Personal' on a 6 GHz network, the service sends 'WPA2-Personal' to firmware. Firmware either rejects the write (USP error) or silently applies an invalid/weaker security config.

Trigger condition: User opens WiFi Advanced settings, selects any non-WPA3 security mode on a 6 GHz band network, saves. saveAdvanced is called → curr.securityMode is written raw.

Fix:

// saveAdvanced — line 433: look up band the same way saveQuickSetup does
final band = bandByApPath[curr.accessPointInstancePath] ?? '';
securityModeEnabled:
    securityChanged && curr.securityMode.isNotEmpty
        ? _securityModeFor6GHz(band: band, selectedMode: curr.securityMode)
        : null,

Note: bandByApPath is already built at the top of saveAdvanced — it just needs to be referenced here.

⚠️ Warning Details

W-1 — [both reviewers] wifiDisplayValue has no 'OWE' case — raw technical token shown in UI (Confidence: 🟢High)

lib/page/_shared/components/wifi_ui.dart:100–110:

String wifiDisplayValue(BuildContext context, String value) {
  switch (value) {
    case 'Auto':  return loc(context).auto;
    case 'None':  return loc(context).none;
    case 'Mixed': return loc(context).mixed;
    default:      return value;  // ← 'OWE' falls here → UI shows "OWE"
  }
}

lib/page/wifi_settings/cards/usp_wifi_status_card.dart:177 calls wifiDisplayValue(context, ap.securityMode) — so after this PR, security-mode displays on the WiFi status card show raw "OWE" instead of a human-readable label like "Enhanced Open".

Fix: Add a case and a localization entry:

case 'OWE': return loc(context).enhancedOpen; // or 'Enhanced Open' as interim

Add "enhancedOpen": "Enhanced Open" to app_en.arb (and other arb files).


W-2 — [both reviewers] isOpenSecurity getter is dead code; open-mode check duplicated (Confidence: 🟢High)

lib/page/wifi_settings/models/wifi_network_ui_model.dart:105–107:

/// 'OWE' is the TR-181 token for Enhanced Open (firmware only accepts 'OWE').
bool get isOpenSecurity =>
    securityMode == 'None' || securityMode.isEmpty || securityMode == 'OWE';

Zero callers in lib/ (grep-verified). Meanwhile lib/page/wifi_settings/models/wifi_settings_settings.dart:55–56 re-implements the same logic inline:

// wifi_settings_settings.dart:55
final isOpen = securityMode == 'None' || securityMode == 'OWE';

And usp_wifi_settings_service.dart:640 has a third independent copy:

const openModes = {'None', 'OWE', ''};

Three sites defining the same set of "open modes" — any future token addition must be updated in all three places.

Fix: Make isOpenSecurity static and have wifi_settings_settings.dart:55 delegate to it:

// wifi_network_ui_model.dart
static bool isOpenSecurityMode(String mode) =>
    mode == 'None' || mode.isEmpty || mode == 'OWE';
bool get isOpenSecurity => isOpenSecurityMode(securityMode);

// wifi_settings_settings.dart:55
final isOpen = WifiNetworkUIModel.isOpenSecurityMode(securityMode);
✅ What looks good
  • Token replacement complete (High): All three production files correctly updated: wifi_network_ui_model.dart:107, wifi_settings_settings.dart:56, usp_wifi_settings_service.dart:640-641'Enhanced-Open''OWE' throughout.
  • Docstring updated (High): _securityModeFor6GHz doc comment at line 630 updated to reflect 'OWE' — no stale 'Enhanced-Open' references remain in HEAD.
  • Test file updated (High): usp_wifi_settings_state_test.dart:537–538 correctly uses 'OWE' in HEAD — no stale 'Enhanced-Open' in test fixtures.
  • New 6 GHz override tests (High): usp_wifi_settings_service_test.dart adds comprehensive saveQuickSetup 6 GHz test group (5 cases: OWE→OWE, None→OWE, WPA3→WPA3, 2.4GHz pass-through, 5GHz pass-through). Tests are well-structured and clearly guard against regression to invalid 'Enhanced-Open' token.
  • Non-6 GHz transparency correct (High): _securityModeFor6GHz returns selectedMode unchanged for non-6GHz bands — 2.4/5GHz behavior unaffected.
  • No hardcoded credentials (High): Changes are limited to security mode string constants — no auth tokens or secrets introduced.
  • isOpenSecurity logic correct (High): The three-way check 'None' || isEmpty || 'OWE' correctly covers all open-mode variants.

Cross-reviewed by two independent agents (security+correctness / architecture+maintainability). Automated — please sanity-check before merge.

@AustinChangLinksys AustinChangLinksys added the need-decide Automated review left findings needing Austin to decide (fix/defer/dismiss). label Jul 15, 2026

@PeterJhongLinksys PeterJhongLinksys left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Automated PR Review

Verdict: 🔴 Comment — unresolved critical issue(s); not approving

Findings

  • 🔴 Critical — none new (the sole critical is the prior saveAdvanced bypass, still open — see "Already raised")
  • 🟡 Warning — Mislabeled/ineffective regression test at test/page/wifi_settings/services/usp_wifi_settings_service_test.dart:1452: the case titled "6 GHz + WPA2-Personal selected → forced to WPA3-Personal" actually passes selectedMode: 'WPA3-Personal', so it only verifies WPA3→WPA3 and would still pass even if the non-open→WPA3 coercion branch were removed. Use 'WPA2-Personal' as input to genuinely guard the override.
  • 🟢 Nit — none

Already raised (skipped to avoid duplication)

  • saveAdvanced writes curr.securityMode verbatim, bypassing _securityModeFor6GHz (usp_wifi_settings_service.dart:433-436) — AustinChangLinksys, still open (PR head is the same commit reviewed; code unchanged). This is the unresolved critical driving the Comment verdict. Note: reachability is bounded — the Advanced-mode dialog populates from each network's own band-specific supportedSecurityModes (wifi_network_card.dart:224), so an invalid 6 GHz mode is only selectable if firmware advertises it (e.g. None on 6 GHz); but the two save paths remaining inconsistent is a real gap worth closing in this PR.
  • wifiDisplayValue has no 'OWE' case → raw "OWE" shown on the status card (wifi_ui.dart:100-110, usp_wifi_status_card.dart:177) — AustinChangLinksys, still open.
  • isOpenSecurity getter has zero lib/ callers; open-mode check duplicated across three sites — AustinChangLinksys, still open (pre-existing; DRY nit).
  • No saveAdvanced + 6 GHz test coverage — AustinChangLinksys, still open.

Verified

  • Token replacement 'Enhanced-Open''OWE' is complete and correct across all three production sites (wifi_network_ui_model.dart:107, wifi_settings_settings.dart:56, usp_wifi_settings_service.dart:639-640); doc comment updated too.
  • saveQuickSetup correctly routes the selected mode through _securityModeFor6GHz before writing Security.ModeEnabled (usp_wifi_settings_service.dart:311-315, 327).
  • USP errors are mapped to ServiceError via mapUspErrorToServiceError() in both saveQuickSetup and saveAdvanced catch blocks — constitution Article XIII respected.
  • SET payload uses the codegen WiFiAccessPointUpdate model (securityModeEnabled); no hand-editing of .g.dart.
  • New isOpenSecurity test group (None / empty / OWE / WPA2 / WPA3-Transition) asserts correct open/non-open classification; other security modes (WPA2/WPA3) are unaffected by the change.
  • The other four new 6 GHz tests (OWE→OWE, None→OWE on 6 GHz; OWE and None written verbatim on 2.4/5 GHz) are correct and meaningful.

saveAdvanced wrote curr.securityMode verbatim while saveQuickSetup routed
it through _securityModeFor6GHz. The two save paths could therefore send
different security modes to firmware, and on 6 GHz saveAdvanced could send
a non-WPA3 (invalid) mode.

- saveAdvanced: route securityModeEnabled through _securityModeFor6GHz
  (band from curr.band), matching saveQuickSetup. Still gated on
  securityChanged so enable-only toggles never re-write the mode.
- wifiDisplayValue: add an 'OWE' case returning 'Enhanced Open' so the
  status card and selector show the Wi-Fi standard label instead of the
  raw token (like WPA2/WPA3-Personal, it is a technical term, not l10n'd).
- tests: the 6 GHz "WPA2-Personal to WPA3-Personal" case passed
  'WPA3-Personal' as input, an identity that would pass even without the
  override; pass 'WPA2-Personal' to exercise the non-open to WPA3 coercion.
  Add a saveAdvanced 6 GHz override group, and use a distinct baseline mode
  so the security diff always fires.
@HankYuLinksys

Copy link
Copy Markdown
Collaborator Author

Pushed 2aa7fe0a addressing the review feedback.

Fixed

  • saveAdvanced 6 GHz override. saveAdvanced now routes securityModeEnabled through _securityModeFor6GHz (band from curr.band), matching saveQuickSetup. Both save paths now write a firmware-valid mode on 6 GHz. Still gated on securityChanged, so enable-only toggles never re-write the mode.
  • wifiDisplayValue OWE label. Added a 'OWE' case returning 'Enhanced Open', so the status card and security-mode selector show the Wi-Fi standard label instead of the raw token. Like WPA2/WPA3-Personal it is a technical term, so it is not localized.
  • Mislabeled 6 GHz test. The WPA2-Personal → WPA3-Personal case was passing 'WPA3-Personal' as input — an identity that would pass even if the override were removed. It now passes 'WPA2-Personal' to genuinely exercise the non-open → WPA3 coercion.
  • saveAdvanced + 6 GHz coverage. Added a saveAdvanced 6 GHz override test group (OWE → OWE, None → OWE, WPA2 → WPA3, 5 GHz verbatim). Reverting the override makes these fail, confirming they guard the fix.

Not changed

  • isOpenSecurity dead code / duplicated open-mode check: left as-is. It is pre-existing and correct; consolidating it is out of scope for this fix.

All wifi_settings tests pass (179); flutter analyze clean.

@AustinChangLinksys AustinChangLinksys left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 Automated Review — Round 2 · 19bc7f0..2aa7fe0 (incremental)

Verdict: ✅ APPROVE — All Round 1 issues resolved; one new low-confidence defensive-gap warning, no blocking issues.

Conf. Where Issue (one-liner)
⚠️ ⚪Low usp_wifi_settings_service.dart:428 isNotEmpty guard short-circuits _securityModeFor6GHz when securityMode == ''; on 6 GHz, empty string maps to 'OWE' inside the function but never reaches it
💡 🟢High usp_wifi_settings_service_test.dart saveAdvanced 6 GHz tests missing WPA3-Personal -> WPA3-Personal passthrough case
💡 🟡Med lib/page/_shared/components/wifi_ui.dart:108 'OWE' -> 'Enhanced Open' mapping has no unit test

Confidence: 🟢High = code-verified · 🟡Med = located + reasoned, not fully confirmed · ⚪Low = speculative, please double-check.
Items marked [both reviewers] were independently flagged by two agents -> higher confidence.

⚠️ Warning Details

W-NEW-1 — isNotEmpty guard silently suppresses OWE coercion when securityMode is empty on 6 GHz (Confidence: ⚪Low)

lib/page/wifi_settings/services/usp_wifi_settings_service.dart:428–433 (head 2aa7fe0):

final securityMode = securityChanged && curr.securityMode.isNotEmpty
    ? _securityModeFor6GHz(
        band: curr.band,
        selectedMode: curr.securityMode,
      )
    : null;

The outer curr.securityMode.isNotEmpty guard was inherited from the original code (which prevented sending '' verbatim to firmware — a valid concern). However, _securityModeFor6GHz (line 647) already handles empty string:

const openModes = {'None', 'OWE', ''};
return openModes.contains(selectedMode) ? 'OWE' : 'WPA3-Personal';

This means an AP whose firmware-stored securityMode is '' on 6 GHz would yield securityModeEnabled: null (nothing sent) instead of 'OWE'. By contrast, saveQuickSetup passes curr.securityMode unconditionally to _securityModeFor6GHz when securityChanged.

Practical impact is low — firmware normally returns a non-empty mode string. This is a defensive gap rather than an active regression, and no existing data flow produces this scenario. The three new tests all use non-empty inputs and pass correctly.

Fix (if desired):

// Let _securityModeFor6GHz handle '' internally:
final securityMode = securityChanged
    ? _securityModeFor6GHz(band: curr.band, selectedMode: curr.securityMode)
    : null;

_securityModeFor6GHz already safely maps '' on non-6 GHz bands (returns '' unchanged), so removing the isNotEmpty guard is safe.

(low confidence — please verify before acting)

✅ What looks good
  • C-1 resolved (High) — saveAdvanced 6 GHz override wired correctly: usp_wifi_settings_service.dart:428–433 now routes curr.securityMode through _securityModeFor6GHz(band: curr.band, ...), and securityModeEnabled receives the coerced value. The raw bypass is gone.
  • W-1 resolved (High) — wifiDisplayValue OWE label added: wifi_ui.dart:108–111 adds case 'OWE': return 'Enhanced Open' — the firmware token is correctly mapped to the Wi-Fi standard label. Comment explains the rationale (technical term, not localized, consistent with WPA2/WPA3-Personal).
  • Suggestion resolved (High) — saveAdvanced 6 GHz tests added: New capturedModeEnabled helper + 4 test cases covering OWE->OWE, None->OWE, WPA2->WPA3-Personal coercion, and 5 GHz verbatim passthrough. All four cases are semantically correct and genuinely guard the fix.
  • W-2 status — pre-existing, author accepted: isOpenSecurity dead code / triple open-mode check duplication acknowledged as pre-existing; left for a future consolidation. Reasonable.
  • Mislabeled saveQuickSetup test fixed (High): usp_wifi_settings_service_test.dart:1517 now passes 'WPA2-Personal' as input instead of the identity 'WPA3-Personal' — test now genuinely exercises the non-open -> WPA3 coercion branch.
  • _securityModeFor6GHz is correct in head (High): openModes = {'None', 'OWE', ''} with return openModes.contains(selectedMode) ? 'OWE' : 'WPA3-Personal' — handles all open-mode variants correctly.
  • Docstring updated (High): usp_wifi_settings_service.dart:636–640 — doc comment correctly describes OWE as the TR-181 token and documents the three routing rules.
  • No hardcoded secrets, no injection vectors (High): Changes limited to string constants and routing logic.

Cross-reviewed by two independent agents (security+correctness / architecture+maintainability). Automated — please sanity-check before merge.

@AustinChangLinksys
AustinChangLinksys merged commit 70c703d into dev-2.6.0 Jul 16, 2026
2 checks passed
@AustinChangLinksys
AustinChangLinksys deleted the fix/wifi-owe-security-mode-1073 branch July 16, 2026 04:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

need-decide Automated review left findings needing Austin to decide (fix/defer/dismiss).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WiFi Settings: Unable to change security mode to Enhanced Open Only

3 participants