Skip to content

fix(auth): token renewal does not check revocation; /renew has no rate limit and its ceiling is undocumented #801

Description

@alissonrosa-lang

Summary

RenewUserToken never consults the user store, so a deactivated or deleted user keeps getting 200 with a fresh token from POST /api/v2/auth/token/renew for up to the max-lifetime ceiling. The CHANGELOG states the opposite. The endpoint also has no rate limiter, and the ceiling that bounds it is undocumented and unreachable via Helm.

Verified against main (c00cc27).

1. Renewal does not check revocation

internal/auth/jwt.go, RenewUserToken validates signature / issuer / audience / expiry and then rebuilds the principal purely from claims:

user := &User{ID: c.Subject, TenantID: c.TenantID, Email: c.Email, Roles: c.Roles}
renewed, err = a.mintUserToken(user, ttl, origin)

Compare Authenticate in the same file, which does the right thing (the #654 fix):

user, active, err := a.store.FindUserByID(ctx, c.Subject)

So renewal is the one auth path that does not reload the user. CHANGELOG.md says:

revocation is still enforced per request, so a renewed token dies the moment its user is deactivated

That is true for useAuthenticate rejects the renewed token everywhere else — and false for issuance: /renew keeps answering 200. It is not an authorization bypass, but it is a credential-issuance oracle for a principal that has already been revoked, and the claim as written will be read as covering both.

None of the renewal tests exercise a store, so nothing would catch a regression here either way.

2. /renew has no rate limiter

In internal/api/server.go, /auth/token is wired with deps.RateLimiter; the /api/v2/auth/token/renew registration passes no limiter. Both are public auth endpoints; the asymmetry looks unintentional.

3. The ceiling is undocumented and Helm-unreachable

auth.jwt.max_lifetime_seconds (default 86400) is what bounds the renewal window, and it is the control that makes the whole feature acceptable. It is registered in serverDefaults, so the env var binds — but it appears in neither website/content/reference/configuration.md nor helm/leoflow/values.yaml. Only extraEnv reaches it.

This is the same class as #725 / #733 / #743, reintroduced by the commit that adds a security surface. And TestDocumentedEnvVarsBind cannot catch it: the guard runs doc → binding, never binding → doc.

Suggested fix

  • Have RenewUserToken reload the user and refuse when inactive/missing (it already takes no ctx — that signature would need to change, the same way Authenticate's did). Failing that, correct the CHANGELOG line so it distinguishes use from issuance.
  • Wire the rate limiter into /renew.
  • Document LEOFLOW_AUTH_JWT_MAX_LIFETIME_SECONDS and give it a chart value.
  • Consider a reverse guard in env_binding_test.go (registered-but-undocumented), which would have caught this and would catch the next one.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions