basecamp / basecamp/basecamp-sdk

Kotlin: enforce the OAuth address policy, settling the CIO-vs-OkHttp engine question (SPEC §16 req 5–6)

Open
#817 0 comments 0 reactions 0 assignees View on GitHub
kotlin security spec
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.