Jon Vanvik
41098831ae
Outcome of a full multi-agent code review. Touches TLS, credential storage, API versioning, networking correctness, concurrency/lifecycle, and the release build. See CHANGELOG.md for the full list and the items to port to the iOS app. Security - Remove untrusted-TLS support: delete ServerTrustPolicy.AllowSelfSignedForHost and its trust-all X509TrustManager; drop the self-signed Settings toggle. - Enforce HTTPS in normalizedBaseUrl (reject http + embedded credentials); add network_security_config (no cleartext, system CAs only) + allowBackup=false. - Never persist passwords at rest: remember-me stores email only; no password pre-fill; clear creds on sign-out / when remember-me is off. - Match CSRF host lookup to OkHttp's host parsing; stop leaking raw exception text. Correctness - Fix API versioning: hardcode v1/v3 per endpoint (docs Sec.4) instead of one broken global apiVersion; remove the version plumbing + Settings field. - Treat "warning" status as success (committed write); fail-fast on a missing pagination cursor; close Response on cancellation; tolerant JSON parse; User-Agent. - Firmware isCompatible fails closed + extra acknowledgment for a non-compatible image. Concurrency / lifecycle - Run network I/O on Dispatchers.IO; stale-result generation guards + rethrow CancellationException in all loads; host AppState in a ViewModel (survive config change); thread-safe cookie jar; reset/upgrade re-entry guard; wire list Retry. Build - Enable R8 minify+shrink with kotlinx.serialization keep rules; drop unused navigation-compose; add the missing .gitignore. NOTE: built/verified by static review only (no Android SDK on the dev machine) — run :app:assembleDebug and a release build before shipping. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
104 lines
5.9 KiB
Markdown
104 lines
5.9 KiB
Markdown
# Handoff — PON Go Android
|
||
|
||
Context for the next session/developer. Pairs with [`README.md`](README.md) and
|
||
[`docs/MCMS_API.md`](docs/MCMS_API.md).
|
||
|
||
## Architecture
|
||
|
||
- **`core/`** — pure Kotlin, no Android UI deps. Mirrors the iOS networking core.
|
||
- JSON: MCMS docs are heterogeneous Mongo documents, so we keep raw
|
||
`kotlinx.serialization` `JsonElement` trees and read fields via the accessors
|
||
in `Json.kt` (`el["key"]`, `.string/.int/.double/.array/.obj`) — the analogue
|
||
of the iOS `JSONValue`. Models (`Models.kt`) are thin wrappers over a
|
||
`JsonElement` with computed properties.
|
||
- `ApiClient` (OkHttp): builds versioned requests, injects `Referer` +
|
||
`X-CSRFToken` (writes), unwraps the `{status,data}` envelope, maps status
|
||
codes to `ApiError`, paginates via the `next` cursor. `Call.await()` bridges
|
||
OkHttp to coroutines with cancellation.
|
||
- Repositories return model wrappers; `McmsConnection` wires it all; auth/cookies
|
||
in `Networking.kt` (`SessionCookieJar` + `SessionStore`), TLS policy in
|
||
`ServerTrust`, secrets in `KeychainStore` (EncryptedSharedPreferences).
|
||
- **`ui/`** — Jetpack Compose. Navigation is **state-based** in `MainActivity`
|
||
(a `List<Screen>` back stack + `BackHandler`), not Navigation-Compose — so
|
||
detail screens and the dashboard drill-downs can take in-memory objects
|
||
directly. Each screen has a plain **state-holder class** (`…Model`) exposing
|
||
`mutableStateOf` fields + `suspend load()/refresh()` — the analogue of the iOS
|
||
`@Observable` view models. No androidx `ViewModel`/DI.
|
||
|
||
## Build / verify loop (this matters)
|
||
|
||
The dev machine had **no `kotlinc`/`gradle` on PATH**, but Android Studio provides
|
||
everything. Two loops:
|
||
|
||
- **Full build (authoritative):**
|
||
```
|
||
export JAVA_HOME="/Applications/Android Studio.app/Contents/jbr/Contents/Home"
|
||
./gradlew :app:assembleDebug --no-daemon --console=plain
|
||
```
|
||
Needs `local.properties` (`sdk.dir=…`, git-ignored). First run downloads deps
|
||
to `~/.gradle`; afterwards it's ~20–60 s. This is what proves the whole app
|
||
(UI + resources + manifest + icons) compiles. **`release` now enables R8
|
||
minify+shrink** (keep rules in `proguard-rules.pro`) — smoke-test a `release` build
|
||
before shipping, since R8 can strip serializers if the keep rules ever drift.
|
||
- **Quick core-only check (no Gradle), when iterating on `core/`:** the bundled
|
||
compiler at `…/Android Studio.app/Contents/plugins/Kotlin/kotlinc/bin/kotlinc`
|
||
with `-Xplugin=…/kotlinc/lib/kotlinx-serialization-compiler-plugin.jar` and a
|
||
classpath of Maven jars (kotlinx-serialization-json/core, okhttp, okio-jvm,
|
||
kotlinx-coroutines-core-jvm). Compiles every `core/*.kt` except
|
||
`KeychainStore.kt` (androidx). Faster than a Gradle round-trip.
|
||
|
||
## API gotchas (all handled in code — don't regress)
|
||
|
||
- **Single-encode URLs.** Ids stay RAW in `McmsEndpoint`; `ApiClient` encodes the
|
||
path once via `HttpUrl.addPathSegments`. Pre-encoding double-encoded MAC colons
|
||
(`:` → `%3A` → `%253A`) → MCMS "invalid id".
|
||
- **Endpoint versions are fixed per endpoint (`v1`/`v3`), NOT global.** Lists/states are
|
||
`v3`; single-doc/login/logs/firmware-files are `v1` (§4) — the two are not
|
||
interchangeable, so there is no global API-version setting. Hardcoded in
|
||
`McmsEndpoint.path()`; endpoints not in the §4 table are marked `// inferred`.
|
||
- **401 bounces to login; 403 does NOT** (permission/CSRF — `requiresReauthentication`
|
||
is 401-only).
|
||
- **Writes return `{"status":"success"}` (or `"warning"`) with no `data`** — the unwrap
|
||
tolerates both. `"warning"` means the write committed (schema mismatch only) and is
|
||
treated as success, NOT an error (§3.5).
|
||
- **ONU registration lives on OLT-STATE buckets** (`OltRepository.registrationMap`),
|
||
not ONU-STATE. Friendly names are `ONU.Name` / `OLT.Name` (NOT `NETCONF.Name`).
|
||
- **CPE DHCP** comes from the web helper `GET /api/cpe/onu/<id>/` — a versionless,
|
||
bare `[code, cpe…]` array (use `ApiClient.getRaw`).
|
||
- **Firmware** = Procedure 7: GET ONU-CFG → write filename/version into the
|
||
**inactive** bank → full-doc PUT (`FirmwareUpgrade.apply`). Inactive-bank only,
|
||
so service stays up until the ONU reboots. `FirmwareFile.isCompatible` now fails
|
||
CLOSED (unknown vendor/model ⇒ not compatible-by-default) and staging a
|
||
non-compatible image needs an extra acknowledgment. Still worth testing on a spare ONU.
|
||
- Field paths were confirmed against live JSON; see `docs/MCMS_API.md` and the
|
||
model accessors.
|
||
|
||
## Trimmed vs iOS (intentional, easy to restore)
|
||
|
||
- ONU detail tabs are **Overview / CPE / Logs / Config** — no separate Alarms tab.
|
||
Current alarms live on ONU-STATE `"Alarm"` buckets if you want to add it.
|
||
- **Config** shows pretty-printed JSON text, not the collapsible tree the iOS app
|
||
has.
|
||
- Dashboard **ONU-state rows aren't tappable** (the Health rows are). The iOS app
|
||
drills ONU-state → filtered ONU list.
|
||
- Toggles use `rememberSaveable` (survive rotation, not process death). Persist in
|
||
prefs if you want parity with iOS `@AppStorage`.
|
||
- **TLS is HTTPS-only with system-CA trust; self-signed/cleartext support was removed**
|
||
(security — see `CHANGELOG.md`, not "easy to restore"). `res/xml/network_security_config.xml`
|
||
forbids cleartext and user-installed CAs. To reach a private-cert box, install its CA in
|
||
the device system store or pin its key via `ServerTrustPolicy.PinPublicKeySha256`. The
|
||
trust policy lives in `ServerTrust`.
|
||
|
||
## Repo split
|
||
|
||
This folder is self-contained (has its own `docs/MCMS_API.md`). To make it a git
|
||
repo: `git init` here; the included `.gitignore` already excludes `build/`,
|
||
`.gradle/`, `local.properties`, `.idea/`. Commit the Gradle wrapper
|
||
(`gradlew`, `gradle/wrapper/*`). The full dev-guide PDFs (`323-1961-30x`) live in
|
||
the iOS repo if you ever need the deep source.
|
||
|
||
## Possible next steps
|
||
|
||
Restore the trimmed items above; add OLT firmware (Procedure 8 bulk task);
|
||
per-ONU live-alarms view; charts for PM history; pull-to-refresh on lists;
|
||
dark-theme polish; a proper app theme from the brand colours.
|