Use expirationDurationInSeconds for token refresh window - #243
Merged
Conversation
The 55-minute refresh window was hardcoded inline in `nextValidToken` while the named `expirationDurationInSeconds` constant sat unused — so changing the constant silently had no effect. Reference the constant directly so it is the single source of truth. Also strengthen the token-manager tests: - `testTokenReusedJustBeforeBoundaryAndRefreshedAtBoundary` pins the `[0, 55min)` reuse window at the exact boundary. - `testRefreshedTokenIsStructurallyValid` decodes the refreshed JWT and asserts the header/claims, rather than relying solely on string inequality (ECDSA signatures are randomized, so a different string alone proves little).
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
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.
Problem
APNSAuthenticationTokenManagerdeclaresexpirationDurationInSeconds = .seconds(60 * 55)but never reads it — the 55-minute refresh window is hardcoded inline innextValidToken. Changing the named constant would silently have no effect, a latent foot-gun.Separately,
testTokenIsRefreshedasserts only that two token strings differ — but ECDSA P256 signatures are randomized, so the strings differ even when the payload is unchanged. The test passes for the wrong reason and proves little about the refresh.Changes
expirationDurationInSecondsin the refresh check so it is the single source of truth.testTokenReusedJustBeforeBoundaryAndRefreshedAtBoundarypins the[0, 55min)reuse window at the exact boundary (reused at 54:59, refreshed at 55:00).testRefreshedTokenIsStructurallyValiddecodes the refreshed JWT and asserts the header/claims (alg,kid,iss), instead of relying on string inequality.Note
I deliberately did not change the JWT
iatfromDispatchWallTimeto the injectedClock:ContinuousClockis monotonic and cannot yield epoch seconds, so wall-clock time is the correct source foriat, while the injected clock correctly measures the refresh interval.Testing
swift test --filter APNSAuthenticationTokenManagerTests— 5 tests pass.