Auth: core building blocks — token generation, repositories, clock#45
Conversation
- TokenGenerator mints 256-bit base64url tokens and stores only SHA-256 hex digests - JdbcClient repositories (first runtime DB access in the repo): magic_link_tokens CRUD with an atomic consume, and the auth domain's minimal view of members incl. race-safe shell insert - Injectable UTC Clock so expiry logic is testable Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
Graphite Automations"Request reviewers once CI passes" took an action on this PR • (07/10/26)2 reviewers were added to this PR based on Henry Chen's automation. |
|
|
||
| /** Invalidates every outstanding link for an email; called before issuing a new one. */ | ||
| public void deleteByEmail(final String email) { | ||
| jdbc.sql("DELETE FROM magic_link_tokens WHERE email = :email") |
There was a problem hiding this comment.
NTS: double check what library we standardize on for SQL
| .update(); | ||
| } | ||
|
|
||
| public void insert(final UUID id, final String email, final String tokenHash, final Instant expiresAt) { |
There was a problem hiding this comment.
Would be good to have a more descriptive name than just insert. insertMagicLinkToken?
| import org.springframework.context.annotation.Bean; | ||
| import org.springframework.context.annotation.Configuration; | ||
|
|
||
| /** Provides the single injectable {@link Clock} so services never call {@code Instant.now()} directly. */ |
| */ | ||
| @Repository | ||
| @RequiredArgsConstructor | ||
| public class MemberAccountRepository { |
There was a problem hiding this comment.
I know that this is required to get things working, but this stuff should be in Allison's domain/work.
| .param("tokenHash", tokenHash) | ||
| .param("now", now.atOffset(ZoneOffset.UTC)) | ||
| .query(String.class) | ||
| .optional(); |
There was a problem hiding this comment.
just a note that this will show the same behavior for emails that don't exist and emails that have already consumed a token (empty return value). this is probably fine as long as we don't just always assume these functions' return values to be not existent emails
There was a problem hiding this comment.
actually now i see here how returning empty for all of these relates to the whole "hiding the email existence" security principle, makes more sense now.
i would add to the javadoc here to explicitly say why an empty value is returned for all error scenarios, it's not clear from just this function.
There was a problem hiding this comment.
also, function should probably be named "consumeAndReturnEmail" or something. the name sounds like it just uses the token.
|
|
||
| /** Case-insensitive lookup: legacy rows may hold mixed-case emails while auth normalizes to lowercase. */ | ||
| public Optional<MemberAccount> findByEmail(final String email) { | ||
| return jdbc.sql(SELECT + " WHERE LOWER(email) = :email") |
There was a problem hiding this comment.
i know that we do normalize the email on the AuthService side, but we shouldn't assume here that it's done in that code. We should normalize both sides if we really want to ensure case insensitivity. (i.e. lowercase and trim for both)
3 participants
hex digests
magic_link_tokens CRUD with an atomic consume, and the auth domain's
minimal view of members incl. race-safe shell insert
Co-Authored-By: Claude Fable 5 noreply@anthropic.com