Skip to content

feat: authenticate /v1, add resolve/client-id, and a deployable chart - #2

Merged
JamesReate merged 1 commit into
mainfrom
feat/v1-auth-and-chart
Aug 7, 2026
Merged

feat: authenticate /v1, add resolve/client-id, and a deployable chart#2
JamesReate merged 1 commit into
mainfrom
feat/v1-auth-and-chart

Conversation

@JamesReate

Copy link
Copy Markdown
Member

Unblocks deploying this service at all. Two things stood in the way: /v1 had no
authentication, and there was no chart.

/v1 was completely unauthenticated

The route comment said it outright — "That middleware is not wired yet; the
route is registered so the contract is visible and testable."
Harmless while
nothing was deployed; the moment it is, anyone able to reach the service can ask
for authorization decisions about any tenant.

Now: jwtware verifies developer-license JWTs against the DIMO JWKS, then a
resolver maps ethereum_address onto a tenant via
tenant_credentials.dimo_client_id.

The order is load-bearing, not cosmetic. Reversed, the claim would be
attacker-supplied and the resolver would authenticate anyone. There's a test
asserting an unauthenticated request cannot reach a handler, because that is the
property that matters.

Two deliberate differences from kaufmann's resolver

kaufmann here why
Match DimoClientID.EQ(addr) lower(dimo_client_id) = lower($1) Exactly the expression the unique index is built on, so lookup and uniqueness agree
Duplicates qm.Limit(1) none kaufmann's schema permits duplicate client ids — its comment says they "shouldn't happen, but the data model allows it", and in production two pairs did. Here the unique index makes a duplicate a schema violation that should surface, not be truncated to whichever row sorted first

GET /v1/resolve/client-id/{clientId}

Listed in the spec; replaces kaufmann's in-app resolver, which stops being able
to answer once an operator's license is shared with its customers. Returns 404
for an unknown id — unlike /v1/authz, where "no access" is a 200, because authz
asks a question with a legitimate negative answer while this dereferences an
identifier that either exists or doesn't.

Chart

Deployment (with the migrate init container), service, config, ExternalSecret,
PDB, service account. Two omissions are choices, not oversights:

  • No ingress. /v1 is cluster-internal by design. Publishing one would make
    a credential check the only thing between the internet and every tenant's
    authorization data.
  • No ServiceMonitor. There is no /metrics endpoint yet, so scraping it
    would create a permanently-failing target and the alert noise with it.

Things found along the way

  • No CI or build workflows existed, so no image was ever built — a chart
    alone would not have been deployable. Added both.
  • Adding lint to a repo that had never run it surfaced 8 pre-existing errcheck
    findings
    . Fixed, so CI is green on its first run rather than red.
  • The Dockerfile ignored the COMMIT_HASH build arg the workflow passes,
    running git rev-parse against whatever .git was in the build context —
    /version would have reported unknown in the shipped image.

Open decision, flagged in app.go rather than silently chosen

Authentication identifies which tenant is calling, but no handler restricts a
caller to asking about its own tenant. Tolerable only because the surface is
cluster-internal. Whether to enforce caller == subject depends on which license
the callers actually present — fleet-lite holds per-tenant credentials and could
present the subject's, which would make enforcement natural. CallerFrom exposes
the caller so a handler can log or enforce it without further plumbing.

Verification

go build, go vet, gofmt, golangci-lint (0 issues) and the full suite pass;
the DB-backed resolver tests ran against the real schema rather than skipping.
helm lint passes and the rendered chart was validated with
kubectl apply --dry-run against the live cluster — all six resources, including
the ExternalSecret CRD.

🤖 Generated with Claude Code

https://claude.ai/code/session_01RifTN8ZabDWb1hW7XtBRcQ

/v1 was completely unauthenticated. The route comment said the middleware
"is not wired yet; the route is registered so the contract is visible and
testable" — fine while nothing was deployed, but it means anyone who could
reach the service could ask for authorization decisions about any tenant.
That was the blocker for deploying at all.

Adds, in the order they matter:

- jwtware verifying developer-license JWTs against the DIMO JWKS, then a
  resolver mapping ethereum_address onto a tenant via
  tenant_credentials.dimo_client_id. Order is load-bearing: reversed, the
  claim would be attacker-supplied and the resolver would authenticate
  anyone.
- GET /v1/resolve/client-id/{clientId}, which the spec lists and which
  replaces kaufmann's in-app resolver — that resolver stops being able to
  answer once an operator's license is shared with its customers.
- A Helm chart: deployment with the migrate init container, service,
  config, ExternalSecret, PDB, service account.

Two differences from kaufmann's resolver, both deliberate. It matches on
lower(dimo_client_id), the exact expression tenant_credentials' unique
index is built on, so lookup and uniqueness agree. And it has no
qm.Limit(1) — kaufmann needs one because its schema permits duplicate
client ids, and in production two pairs existed; here the unique index
makes a duplicate a schema violation that should surface rather than be
truncated to whichever row sorted first.

Chart decisions worth knowing:

- No ingress. /v1 is cluster-internal by design; publishing one would make
  a credential check the only thing between the internet and every
  tenant's authorization data.
- No ServiceMonitor. There is no /metrics endpoint yet, so scraping would
  create a permanently-failing target and the alerts that follow.

Also adds the CI and build workflows, which did not exist — so no image
was ever built and a chart alone would not have been deployable. Adding
lint to a repo that had never run it surfaced 8 pre-existing errcheck
findings; fixed, so CI is green from the first run rather than red.

Fixes the Dockerfile to use the COMMIT_HASH build arg the workflow passes.
It was running `git rev-parse` against whatever .git was in the build
context, so /version would have reported "unknown" in the shipped image.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RifTN8ZabDWb1hW7XtBRcQ
@JamesReate
JamesReate merged commit de565ee into main Aug 7, 2026
2 checks passed
@JamesReate
JamesReate deleted the feat/v1-auth-and-chart branch August 7, 2026 12:45
JamesReate added a commit that referenced this pull request Aug 7, 2026
#2 added only lint and buildpushprod. Comparing against kaufmann-oracle
and fleet-lite-app, four pieces were missing — two of them load-bearing
rather than cosmetic.

helmlint: #2 added a chart and nothing in CI validated it. That is the gap
that forced the chart to be checked by hand with helm template and
kubectl --dry-run before merging. Mirrors kaufmann's job, plus a step that
renders values-prod.yaml — the file ArgoCD actually deploys and the one
the release workflow rewrites, which the upstream job leaves unchecked.

buildpushdev: without it the Docker build first runs at release-tag time,
so a broken Dockerfile blocks a release rather than a PR. That is exactly
how #2's COMMIT_HASH bug would have surfaced. Building on every merge to
main also keeps a current image available for whenever a dev environment
exists.

dependabot.yml and PULL_REQUEST_TEMPLATE.md: straight parity with
kaufmann-oracle.

lint.yml keeps fleet-lite's shape, with lint and test as two jobs in one
file, rather than kaufmann's split into lint.yml plus test.yml. Both
patterns exist in the org; this repo already follows the former.

Verified rather than assumed: the helm template step runs clean, and the
image builds with the build arg actually embedded — extracted the binary
and confirmed the ldflags value is present, since nothing had ever built
this Dockerfile before.


Claude-Session: https://claude.ai/code/session_01RifTN8ZabDWb1hW7XtBRcQ

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant