basecamp / basecamp/basecamp-sdk
Kotlin: enforce the OAuth address policy, settling the CIO-vs-OkHttp engine question (SPEC §16 req 5–6)
- Dominant language
- Go
- Stars
- 49
- Forks
- 12
- Avg merge
- 20h 47m
- Merged PRs (30d)
- 89
Description
Extend SPEC §16 requirements 5–6 (dial-time address judgement of the advertised issuer, `token_endpoint`, and `device_authorization_endpoint`) to the Kotlin SDK. Appendix F records today's state: scheme gate, bounded timeout, `followRedirects = false`, bounded body — a private address is still dialed.
**Step 0 (upstream, blocking):** a Kotlin surfguard classification-core port in the surfguard repo (classification + resolve, IANA generator over `script/iana/*.json`, mirrored 157-case corpus with byte-equality drift tests). Tracked as basecamp/surfguard#25.
**Enforcement design and the open question to settle first:**
- The JVM seam is OkHttp's `Dns` interface: OkHttp connects to exactly the addresses `Dns.lookup` returns, so filtering/judging there IS connect-time judgement — no second resolution, which is the DNS-rebinding requirement (the address judged MUST be the address connected).
- **The engine conflict:** the SDK ships the CIO engine today, which exposes no equivalent hook. Options to evaluate: (a) move the OAuth surface (or all of the JVM target) to the OkHttp engine and enforce via `Dns`; (b) keep CIO for API traffic and carry OAuth requests on a dedicated OkHttp-engined client; (c) accept a JVM-only enforcement tier with OkHttp and document other targets (Native/JS, if ever shipped) as unenforced. Multiplatform has no portable dial hook — the chosen answer must state the per-target story explicitly in Appendix F.
- The exchange/device paths already re-wrap injected engines (`hardenedDeviceClient`/`hardenedTokenClient`); the policed default client slots into the same construction sites, and the override surface attaches there.
- **Loopback is explicit operator configuration:** a policy option on the OAuth entry points (mirroring Go's `AllowLoopback`), never inferred from the endpoint URL or other request-derived values. The existing localhost HTTPS exemption is a scheme judgement, not address admission.
- **Overrides (SPEC §16 contract):** replacement policy, replacement client, disablement (a caller-supplied client counts — "yours, enforcement included"; note the engine re-wrap preserves redirect suppression and timeouts on injected clients but must NOT layer the address policy onto them).
- **Verdicts:** `invalid_issuer_origin` on the discovery hop, `api_error` elsewhere, non-retryable, device poll terminates on first refusal; unresolvable stays a retryable network fault.
**Definition of done:** Appendix F rows updated; unit tests assert refusal-before-dial for private/loopback/CGNAT/special-purpose targets on discovery, device, and exchange paths, plus override and injected-client contracts, on the enforced (JVM/OkHttp) tier.
Contributor guide
Research direction
Start with the blocking surfguard classification-core port tracked in surfguard#25, then evaluate the CIO and OkHttp engine options at the hardenedDeviceClient and hardenedTokenClient construction sites. Update Appendix F with the per-target decision and run unit tests covering refusal-before-dial, overrides, and injected-client contracts on the enforced JVM/OkHttp tier.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- kotlin
- Domain
- api, authentication, security
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100