fix(rsky-pds): getServiceAuth exp is seconds, not milliseconds#207
Open
rudyfraser wants to merge 2 commits into
Open
fix(rsky-pds): getServiceAuth exp is seconds, not milliseconds#207rudyfraser wants to merge 2 commits into
rudyfraser wants to merge 2 commits into
Conversation
exp is a Unix timestamp in seconds (per the lexicon) but was multiplied by 1_000 (milliseconds) before from_micros_to_utc, which since #206 correctly interprets its argument as microseconds. The 1000x-too-small value landed near 1970, so every getServiceAuth with a requested exp failed BadExpiration "expiration is in past". Convert with 1_000_000 and extract the bounds check into a testable helper with regression tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #206.
getServiceAuthconverted itsexpparam (a Unix timestamp in seconds, per the lexicon) withexp * 1_000(milliseconds) before passing it tofrom_micros_to_utc. Since #206 that function correctly interprets its argument as microseconds, so the value was 1000x too small, landed near 1970, and every request with a requestedexpfailedBadExpiration: expiration is in past. This converts with1_000_000and extracts the bounds check into a pure, testable helper.Note (separate, not fixed here)
inner_get_service_authvalidates the requestedexpbut then passesexp: Nonetocreate_service_jwt, so the minted token always uses the default 60s lifetime regardless of the (now correctly validated) request. Left for a separate change to keep this scoped to the unit bug.Test plan
cargo test -p rsky-pds --lib get_service_auth— 5 regression tests (valid 30-min and sub-minute method-less expiries accepted; past, >1h, and >1min-method-less rejected). All fail on the previous* 1_000and pass on the fix.