feat(otp): add one_time_tokens query helpers - #2797
Merged
Merged
Conversation
CreateOneTimeToken now takes the validity window and stores expires_at on insert. Call sites pass Mailer.OtpExp for email flows and Sms.OtpExp for phone flows, matching the windows the verify path infers from users.*_sent_at today. Reads are unchanged. Adds OtpExpAsDuration helpers on the mailer and SMS configurations. Existing tests are updated for the new signature.
Add a models suite covering persistence, past windows, and resend replacement, and an api suite asserting expires_at equals sent_at + OtpExp per channel. The api suite pins Mailer.OtpExp and Sms.OtpExp to different values so a swapped config field fails the test.
…-start-writing-to-one_time_tokensexpiresat
The test came in from master via merge after this branch added the validityDuration parameter, so the models package did not compile. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…-start-writing-to-one_time_tokensexpiresat
annabkr
marked this pull request as draft
September 9, 2026 16:15
annabkr
force-pushed
the
annabaker/auth-1553-ott-query-helpers
branch
from
September 9, 2026 19:08
fa21e86 to
7577774
Compare
…yRelatesTo Two lookups for the OTT-as-source-of-truth work. FindOneTimeTokenWithPKCEFallback accepts a token hash or its pkce_-prefixed form in a single query and prefers the exact match. FindOneTimeTokenByRelatesTo returns the newest row for a relates_to value and token type. relates_to is not unique for PhoneChangeToken, so the doc comment tells callers to check the user against the request.
models cannot import api, so the constant moves down and api.PKCEPrefix points at it. Replaces the bare "pkce_" literals in the one_time_tokens finders with the constant.
FindOneTimeToken and FindOneTimeTokenWithPKCEFallback delegate to findOneTimeToken, which takes a pkceFallback flag. With the flag off it builds the statement FindOneTimeToken built before: same WHERE string, same args in the same order, same Eager connection, no ORDER BY. The flag only switches the token_hash clause and adds the ORDER BY.
Pin the exact-hash path: no pkce_ match, the type filter applies, and either of two types is found. seedToken replaces the truncate, create user, insert row boilerplate in the hash lookup tests.
annabkr
force-pushed
the
annabaker/auth-1553-ott-query-helpers
branch
from
September 9, 2026 19:14
7577774 to
fc7f405
Compare
annabkr
marked this pull request as ready for review
September 9, 2026 19:15
xlgmokha
approved these changes
Sep 9, 2026
xlgmokha
left a comment
xlgmokha
left a comment
Contributor
There was a problem hiding this comment.
The changes LGTM.
Note: I don't have a lot of experience with this ORM so I think it would be helpful to me to see the SQL queries that are generated and the explain plan for them.
Base automatically changed from
annabaker/auth-1572-start-writing-to-one_time_tokensexpiresat
to
master
September 18, 2026 00:52
…-ott-query-helpers # Conflicts: # internal/api/admin_test.go # internal/api/external_test.go # internal/api/invite_test.go # internal/api/mail.go # internal/api/one_time_token_expiry_test.go # internal/api/phone.go # internal/api/resend_test.go # internal/api/signup_test.go # internal/api/verify_test.go # internal/models/one_time_token.go # internal/models/one_time_token_test.go # internal/models/user_test.go
Contributor
Author
I agree. I actually thought about adding unit tests for the queries, which is a pattern I've appreciated before. However, it would have required a large diversion from established pattens in this repo, I decided to hold off because of that and the anticipated discussion. The queries are pretty simple: -- FindOneTimeToken:
SELECT ... FROM one_time_tokens WHERE token_type = $1 and token_hash = $2 LIMIT 1
-- FindOneTimeTokenWithPKCEFallback:
SELECT ... WHERE token_type = $1 and token_hash in ($2, $3) ORDER BY token_hash = $4 desc LIMIT 1
-- FindOneTimeTokenByRelatesTo:
SELECT ... WHERE token_type = $1 and relates_to = $2 ORDER BY created_at desc LIMIT 1I'll need to look into how to run |
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
2 participants
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.
What kind of change does this PR introduce?
Feature
What is the current behavior?
There is no single-query lookup for a token hash and its
pkce_-prefixed form, and no lookup byrelates_to.Callers such as
FindUserByEmailChangeCurrentAndAudiencerun twoFindOneTimeTokenqueries to cover the prefix, which is inefficient.What is the new behavior?
Adds two parent query methods used by the upcoming OTT-as-source-of-truth work:
FindOneTimeTokenWithPKCEFallback: single-query lookup that accepts either an exact token hash or itspkce_-prefixed form, preferring the exact match.FindOneTimeTokenByRelatesTo: looks up the newest token row for a givenrelates_tovalue (e.g. phone number) and token type.Along with:
PKCEPrefixis now a constant inmodelsthat we reference vs. string literalsFindOneTimeTokenandFindOneTimeTokenWithPKCEFallbacknow sharefindOneTimeToken, guided by apkceFallbackflag.Adds tests for those methods. Not updating existing callers for now to reduce the radius of this change.
Additional context
Contributes to AUTH-1553 but does not close.