Code review fixes: app 2.9.0, server 2.2.0
Telemetry no longer loses queued batches. The server drops a bad value, or a sample without a plausible time, instead of rejecting the batch, and answers 200 with what it dropped. The app sends only values inside the server's ranges (speed as a magnitude, so reversing is fine) and no samples before the head unit's clock is set. Server: - Pairing hands out and accepts a token only after the device is saved; a failed save leaves the request pending. - A paired token serves only the installation id it was paired with (403 otherwise); the shared INGEST_TOKEN still serves any. - Wrong-password lockouts and open pairing requests are limited per client address (last X-Forwarded-For entry), so a stranger cannot lock the owner out; a car starting again replaces its own request. - ABRP forwarding skips samples stamped ahead of the server's clock, which used to stop it until a restart. App: - Settings written to plaintext during a keystore failure move back into encrypted storage; the stored installation id wins. - The installation id is derived from ANDROID_ID on first start, so a reinstall finds its history; a stored or imported id still wins. - The scanned config file is deleted after import (it holds the token). - GPS stops while live telemetry is off or the server unconfigured, and pauses while parked and idle, keeping the fix the car parked with. - Range estimates use the monotonic clock. - One HTTP helper (ServerHttp) for telemetry, sync and pairing. - Last Turkish strings translated; mise logs shows the NilsMG4 tags. Grafana reads InfluxDB with its own read-only token (new grafana-influx Secret, created by hand as described in k8s/README.md). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QLJfZQZeAKAwSqViTmLzki
This commit is contained in:
+100
-3
@@ -34,10 +34,19 @@ func poll(t *testing.T, h http.Handler, deviceCode string) (int, map[string]stri
|
||||
}
|
||||
|
||||
func submit(t *testing.T, h http.Handler, code, password, action string) *httptest.ResponseRecorder {
|
||||
t.Helper()
|
||||
return submitFrom(t, h, "", code, password, action)
|
||||
}
|
||||
|
||||
// submitFrom submits the approval form as if from forwardedFor, the address Traefik adds.
|
||||
func submitFrom(t *testing.T, h http.Handler, forwardedFor, code, password, action string) *httptest.ResponseRecorder {
|
||||
t.Helper()
|
||||
form := url.Values{"code": {code}, "password": {password}, "action": {action}}
|
||||
req := httptest.NewRequest("POST", "/pair", strings.NewReader(form.Encode()))
|
||||
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
|
||||
if forwardedFor != "" {
|
||||
req.Header.Set("X-Forwarded-For", forwardedFor)
|
||||
}
|
||||
rec := httptest.NewRecorder()
|
||||
h.ServeHTTP(rec, req)
|
||||
return rec
|
||||
@@ -86,7 +95,7 @@ func TestPairingHappyPath(t *testing.T) {
|
||||
}
|
||||
// And it works on the ingest endpoints.
|
||||
rec := do(t, h, "POST", "/v1/telemetry", got["token"], string(telemetryBody(`{"utc":1790000000,"soc":80}`)))
|
||||
if rec.Code != 204 {
|
||||
if rec.Code != 200 {
|
||||
t.Fatalf("paired token on telemetry: %d %s", rec.Code, rec.Body)
|
||||
}
|
||||
}
|
||||
@@ -107,6 +116,11 @@ func TestPairingWrongPasswordAndLockout(t *testing.T) {
|
||||
if status, _ := poll(t, h, deviceCode); status != 202 || len(store.devices) != 0 {
|
||||
t.Fatal("a locked-out attempt changed the request")
|
||||
}
|
||||
// The lockout is per address: someone guessing elsewhere does not lock the owner out.
|
||||
if rec := submitFrom(t, h, "198.51.100.7", userCode, testApprovalPassword, "deny"); rec.Code != 200 {
|
||||
t.Fatalf("other address locked out too: %d", rec.Code)
|
||||
}
|
||||
deviceCode, userCode, _ = startPairing(t, h)
|
||||
// After the window the right password works again.
|
||||
api.Pairing.Now = func() time.Time { return time.Now().Add(failedLoginsWindow + time.Second) }
|
||||
if rec := submit(t, h, userCode, testApprovalPassword, "approve"); rec.Code != 200 {
|
||||
@@ -186,7 +200,7 @@ func TestRevokedTokenIsRejected(t *testing.T) {
|
||||
t.Fatalf("revoked token accepted: %d", rec.Code)
|
||||
}
|
||||
// The shared ingest token is unaffected.
|
||||
if rec := do(t, h, "POST", "/v1/telemetry", testToken, string(telemetryBody(`{"utc":1790000000,"soc":1}`))); rec.Code != 204 {
|
||||
if rec := do(t, h, "POST", "/v1/telemetry", testToken, string(telemetryBody(`{"utc":1790000000,"soc":1}`))); rec.Code != 200 {
|
||||
t.Fatalf("shared token: %d", rec.Code)
|
||||
}
|
||||
}
|
||||
@@ -195,7 +209,9 @@ func TestPairedDevicesSurviveARestart(t *testing.T) {
|
||||
devices := []Device{{ID: "d1", TokenHash: hashToken("restored-token-0123456789abcdef"), CreatedAt: time.Now()},
|
||||
{ID: "d2", TokenHash: hashToken("revoked-token-0123456789abcdef"), CreatedAt: time.Now(), Revoked: true}}
|
||||
p := NewPairing(testApprovalPassword, devices)
|
||||
if !p.Authorized("restored-token-0123456789abcdef") || p.Authorized("revoked-token-0123456789abcdef") {
|
||||
_, restored := p.Authorized("restored-token-0123456789abcdef")
|
||||
_, revoked := p.Authorized("revoked-token-0123456789abcdef")
|
||||
if !restored || revoked {
|
||||
t.Fatal("restored devices not honoured")
|
||||
}
|
||||
}
|
||||
@@ -232,3 +248,84 @@ func TestApprovalPasswordMustBeLongEnough(t *testing.T) {
|
||||
}
|
||||
|
||||
var regexpUserCode = regexp.MustCompile(`^[` + userCodeAlphabet + `]{3}-[` + userCodeAlphabet + `]{3}$`)
|
||||
|
||||
func TestPairedTokenOnlyServesItsOwnInstallation(t *testing.T) {
|
||||
api, _ := newTestAPI()
|
||||
h := api.Routes()
|
||||
deviceCode, userCode, _ := startPairing(t, h)
|
||||
submit(t, h, userCode, testApprovalPassword, "approve")
|
||||
_, got := poll(t, h, deviceCode)
|
||||
token := got["token"]
|
||||
other := "7e2b3c4d-5e6f-4a70-9b8c-1d2e3f4a5b6c"
|
||||
body := `{"installationId":"` + other + `","samples":[{"utc":1790000000,"soc":5}]}`
|
||||
if rec := do(t, h, "POST", "/v1/telemetry", token, body); rec.Code != 403 {
|
||||
t.Fatalf("telemetry for another car: %d", rec.Code)
|
||||
}
|
||||
if rec := do(t, h, "GET", "/v1/consumption/history/"+other, token, ""); rec.Code != 403 {
|
||||
t.Fatalf("another car's history: %d", rec.Code)
|
||||
}
|
||||
if rec := do(t, h, "GET", "/v1/consumption/history/"+testInstallation, token, ""); rec.Code != 200 {
|
||||
t.Fatalf("own history: %d", rec.Code)
|
||||
}
|
||||
// The shared token serves any installation.
|
||||
if rec := do(t, h, "POST", "/v1/telemetry", testToken, body); rec.Code != 200 {
|
||||
t.Fatalf("shared token: %d", rec.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestATokenIsCollectableOnlyOnceSaved(t *testing.T) {
|
||||
p := NewPairing(testApprovalPassword, nil)
|
||||
r, err := p.Start(testInstallation, "192.0.2.1")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
d, err := p.Decide(r.userCode, testApprovalPassword, "192.0.2.1", true)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
// Saving is in flight: the car still waits, and the token is not accepted yet.
|
||||
if state, token, _, _ := p.Poll(r.deviceCode); state != pairingPending || token != "" {
|
||||
t.Fatalf("collectable while saving: %v %q", state, token)
|
||||
}
|
||||
p.Forget(r.userCode, d)
|
||||
if _, err := p.Decide(r.userCode, testApprovalPassword, "192.0.2.1", true); err != nil {
|
||||
t.Fatalf("not approvable again after a failed save: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPairingStartsAreLimitedPerAddress(t *testing.T) {
|
||||
p := NewPairing(testApprovalPassword, nil)
|
||||
ids := []string{"6f1a2b3c-4d5e-4f60-8a9b-0c1d2e3f4a51", "6f1a2b3c-4d5e-4f60-8a9b-0c1d2e3f4a52",
|
||||
"6f1a2b3c-4d5e-4f60-8a9b-0c1d2e3f4a53", "6f1a2b3c-4d5e-4f60-8a9b-0c1d2e3f4a54"}
|
||||
for _, id := range ids[:maxPendingPerClient] {
|
||||
if _, err := p.Start(id, "203.0.113.9"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
if _, err := p.Start(ids[3], "203.0.113.9"); !errors.Is(err, errTooMany) {
|
||||
t.Fatalf("over the per-address limit: %v", err)
|
||||
}
|
||||
// The car on its own address is unaffected, and starting again replaces its request.
|
||||
first, err := p.Start(testInstallation, "192.0.2.1")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if _, err := p.Start(testInstallation, "192.0.2.1"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if _, _, _, err := p.Poll(first.deviceCode); !errors.Is(err, errUnknownCode) {
|
||||
t.Fatal("the replaced request is still open")
|
||||
}
|
||||
}
|
||||
|
||||
func TestClientAddressTakesTheProxysEntry(t *testing.T) {
|
||||
req := httptest.NewRequest("GET", "/", nil)
|
||||
req.Header.Set("X-Forwarded-For", "10.9.9.9, 198.51.100.7")
|
||||
if got := clientAddress(req); got != "198.51.100.7" {
|
||||
t.Fatalf("got %q", got)
|
||||
}
|
||||
req.Header.Del("X-Forwarded-For")
|
||||
if got := clientAddress(req); got != "192.0.2.1" {
|
||||
t.Fatalf("without a proxy: %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user