Skip to content

Reject invalid personal access token expiration before issuance - #1084

Open
Floating-Y wants to merge 1 commit into
deeplethe:devfrom
Floating-Y:fix/token-expiry-validation
Open

Floating-Y wants to merge 1 commit into
deeplethe:devfrom
Floating-Y:fix/token-expiry-validation

Conversation

@Floating-Y

Copy link
Copy Markdown
Contributor

Why

POST /api/v1/me/tokens silently treats negative expires_in_days values as permanent tokens. Large positive values can also panic when constructing a duration or adding it to the current date. Invalid expiration requests should return a validation error before issuing a token.

What changes

  • Reject negative values through the existing AppError::invalid response format (HTTP 422, code bad_token_expiry).
  • Use Duration::try_days and checked_add_signed to handle duration overflow and date overflow separately, before token generation, storage, or the token.issued success audit.
  • Preserve the 90-day default, explicit zero for permanent tokens, and all representable positive values. Keep the request field type, permissions, scopes, and response structure unchanged; add no arbitrary expiration limit, dependency, or migration.
  • Add fixed-clock tests for the default, zero, positive values, negatives, both overflow paths, and the representable date boundary. Authenticated route tests cover omitted expiration, 0, 1, 365, -1, i64::MIN, 100000000, and i64::MAX, including unchanged token and success-audit counts after each rejected request.

How it was checked

  • cargo test --locked -p utopia-server token_routes -- --nocapture: all four new regression tests passed, including the HTTP/database test against a dedicated PostgreSQL instance with UTOPIA_TEST_REQUIRE_DB=1.
  • cargo fmt --all --check: passed.
  • cargo clippy --workspace --all-targets -- -D warnings: passed.
  • cargo test --workspace --no-fail-fast -- --test-threads=1: completed successfully against a freshly migrated dedicated test database, with UTOPIA_TEST_REQUIRE_DB=1 and UTOPIA_TEST_REQUIRE_PDFTOTEXT=1.
  • pnpm install --frozen-lockfile and pnpm build in web/: passed.

Validation limits: the default parallel workspace run encountered a database deadlock in the existing failed_fallback_save_never_reports_done chat persistence test; the full serial run passed. Local Windows proxy/localhost connection issues were addressed only in the test environment with NO_PROXY=localhost,127.0.0.1,::1 and an IPv4 database address. Six explicitly ignored tests (live OCR/RSS and opt-in delivery regressions) were not run. Unconfigured MySQL, Trino, and WebDAV live-service checks returned early and are not claimed as validated. The relevant PostgreSQL token tests did execute.

Before review

  • Every commit is signed off (git commit -s).
  • cargo fmt --all --check and cargo clippy --workspace --all-targets -- -D warnings pass.
  • The full workspace suite passes with --test-threads=1; the parallel-run failure and skipped checks are disclosed above.
  • The PR targets dev.

Signed-off-by: Floating-Y <118035379+Floating-Y@users.noreply.github.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