Skip to content

fix: Take related timestamps from a single clock read - #144

Merged
patchandthat merged 2 commits into
RockSolidKnowledge:mainfrom
oleks9o6:fix/133-single-clock-read
Oct 29, 2026
Merged

patchandthat merged 2 commits into
RockSolidKnowledge:mainfrom
oleks9o6:fix/133-single-clock-read

Conversation

@oleks9o6

@oleks9o6 oleks9o6 commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Description

Values that describe the same moment now come from one clock read, so they can't drift apart when a second ticks over between reads.

  • TokenExtensions.CreateJwtPayload: nbf and exp, the case in Time drift in nbf/exp claims #133, where a token could live one second longer than its lifetime.
  • DefaultTokenService: the iat claim and CreationTime of identity and access tokens.
  • ServerSessionTicketStore: a session's Created and Renewed when the ticket has no IssuedUtc, for new and overwritten sessions.
  • SecretValidator: the expired and current secret lists read the clock once per secret, so a secret expiring between reads ended up in neither list.
  • DistributedDeviceFlowThrottlingService: the stored last-seen time, the polling comparison and the cache entry's expiry.

Closes #133

Type of change

  • Bug fix
  • Feature
  • Refactoring
  • Documentation
  • Other

Does this PR introduce a breaking change?

Does the change cause a breaking change to any of the following?

  • Changes to existing public APIs or DB schema
  • Changes which could cause existing behaviour to fail (eg. feature restrictions, changes to default values, stricter validation, etc)

Testing

Each change has a unit test with FakeTimeProvider.AutoAdvanceAmount set, so every clock read moves time on by a second. All seven fail with separate reads and pass with one. The full unit and integration suites pass.

LLM Usage

The src/ changes are written by me. The tests were drafted with Claude and reviewed by me.

Other context

Checked and left as they are, because the reads are separate checks or separate operations: the two authorization code expiry checks in TokenRequestValidator, the max_age and user SSO lifetime checks, PAR storage and validation, refresh token creation, validation and update, and consent and interaction messages.

Not changed here:

  • The back-channel logout token gets iat in DefaultBackChannelLogoutService, while nbf and exp come later from CreateJwtPayload. Aligning them would mean taking nbf and exp from token.CreationTime, which changes tokens created without one, so I've left that to you.
  • A few places read DateTime.UtcNow instead of the TimeProvider. That's a different problem from the drift here, so it has its own issue, Some code reads DateTime.UtcNow instead of the TimeProvider #145.

🤖 Generated with Claude Code

oleks9o6 and others added 2 commits October 28, 2026 14:08
@oleks9o6
oleks9o6 marked this pull request as ready for review October 28, 2026 13:11
@patchandthat
patchandthat self-requested a review October 29, 2026 11:04

@patchandthat patchandthat left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks

@patchandthat
patchandthat merged commit f40ebc7 into RockSolidKnowledge:main Oct 29, 2026
3 checks passed
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.

Time drift in nbf/exp claims

2 participants