Skip to content

fix(auth): extend login verification code TTL to 30 minutes - #70756

Merged
yasen-posthog merged 2 commits into
masterfrom
yasen/fix/code-verification-ttl-30min
Jul 15, 2026
Merged

fix(auth): extend login verification code TTL to 30 minutes#70756
yasen-posthog merged 2 commits into
masterfrom
yasen/fix/code-verification-ttl-30min

Conversation

@yasen-posthog

Copy link
Copy Markdown
Contributor

Problem

The 6-digit login verification code expired after 10 minutes, which was too tight for users switching to their email and back.

Changes

  • Extend the login verification code lifetime from 10 to 30 minutes
  • Derive the verification email's expiry copy from the same TTL constant so the two can't drift

How did you test this code?

  • Existing code-based verification suite passes, including the expiry test updated for the new TTL
  • Agent-authored; only the automated tests above were run.

Automatic notifications

  • Publish to changelog?
  • Alert Sales and Marketing teams?

Bump CODE_TTL_SECONDS from 10 to 30 minutes and derive the email's
expiration copy from the same constant so the two can't drift.
@greptile-apps

greptile-apps Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Security Review

The login-code lifetime is longer. One session-lifetime mismatch can make the auth flow reject codes that are still inside the advertised validity window.

Reviews (1): Last reviewed commit: "fix(auth): extend login verification cod..." | Re-trigger Greptile

Comment thread posthog/helpers/two_factor_session.py
Comment thread posthog/helpers/two_factor_session.py
@yasen-posthog yasen-posthog added the stamphog Request AI approval (no full review) label Jul 14, 2026
@stamphog

stamphog Bot commented Jul 14, 2026

Copy link
Copy Markdown

Note

🤖 stamphog reviewed ebf8677c0835dcc88b76177b5bbdd711594d0b92 — verdict: REFUSED

Gates denied this auth-flow change, and Greptile flagged an unresolved P1 security issue (session TTL mismatch can reject valid codes / extend stale lockout windows) that hasn't been addressed.

  • greptile-apps[bot] reviewed the current head.
  • Deny-list gate failed (auth) and tier gate failed (T2-never)
  • Unresolved P1 security comment from @greptile-apps on posthog/helpers/two_factor_session.py: session lifetime shorter than new 30-minute TTL can cause valid codes to be rejected
  • Unresolved P2 comment: stale failed-attempt lockout counter can persist longer, risking false lockouts
Gate mechanics and policy version
Gate Result
prerequisites all clear
deny-list matches: auth
size 5L, 2F substantive, 7L/3F incl. docs/generated/snapshots — within ceiling
tier classified as T2-never: T2-never (7L, 3F, single-area, fix)
stamphog 2.0.0b3 .stamphog/policy.yml @ 96d3641 · reviewed head ebf8677

@stamphog stamphog Bot removed the stamphog Request AI approval (no full review) label Jul 14, 2026
@greptile-apps

greptile-apps Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

SESSION_COOKIE_AGE isn't set anywhere in posthog/settings/, which means PostHog uses Django's default of 2 weeks (1,209,600 s). That makes my P1 concern inapplicable as filed: the unauthenticated session holding code_based_verification_pending_user_id / code_based_verification_issued_at will never expire in under 30 minutes under standard PostHog configuration. I'll close P1.

P2 is real but pre-existing — the _reset_attempts delete already failed silently before this PR (the exception is caught and logged). What this PR changes is that a stale attempt counter now survives for 30 minutes instead of 10 before its Redis TTL naturally expires it. That's a 3× longer window in which a near-lockout counter from a previous login attempt could ghost a fresh legitimate login. Worth addressing, but it's not a blocker introduced solely by this change.

Concrete fix for P2: replace the expire call in reserve_attempt with a TTL pinned to the remaining code lifetime at the time of first increment, so the counter always expires with the code regardless of whether _reset_attempts succeeds:

    def reserve_attempt(self, request: HttpRequest) -> int:
        """Atomically count this verification attempt against the pending login's budget.

        Returns the running attempt total (including this one) so the caller can reject once it
        exceeds CODE_MAX_ATTEMPTS. Backed by a Redis INCR keyed on the pending user id, so parallel
        guesses can't all read the same pre-increment value and slip past the cap. Fails open on a
        Redis error (returns 0) to keep login working - the per-user verify throttle is the backstop.
        """
        user_id = self.get_pending_code_based_verification_user_id(request)
        if not user_id:
            return 0
        issued_at = self.get_pending_code_based_verification_issued_at(request)
        remaining_ttl = max(1, CODE_TTL_SECONDS - (int(time.time()) - (issued_at or 0)))
        try:
            client = get_client()
            key = self._attempts_redis_key(user_id)
            count = int(client.incr(key))
            # Only set the TTL on first increment; subsequent attempts must not extend it,
            # otherwise the counter can outlive the code.
            if count == 1:
                client.expire(key, remaining_ttl)
            return count
        except Exception:
            mfa_logger.exception(
                "Failed to reserve code-based verification attempt; allowing (throttle still applies)",
                user_id=user_id,
            )
            return 0

This pins the counter's expiry to the remaining code lifetime rather than resetting it on every guess, so even if _reset_attempts fails, the old counter can never outlive the old code.

@trunk-io

trunk-io Bot commented Jul 14, 2026

Copy link
Copy Markdown

Static BadgeStatic BadgeStatic BadgeStatic Badge

View Full Report ↗︎Docs

@github-actions

Copy link
Copy Markdown
Contributor

🤖 CI report

Playwright — all passed

All tests passed.

View test results →

Generated-By: PostHog Code
Task-Id: 1ff694e9-e69e-4c23-807c-125c7b2de4e1
@yasen-posthog
yasen-posthog enabled auto-merge (squash) July 15, 2026 12:07
@yasen-posthog
yasen-posthog merged commit 9572937 into master Jul 15, 2026
241 checks passed
@yasen-posthog
yasen-posthog deleted the yasen/fix/code-verification-ttl-30min branch July 15, 2026 12:26
@deployment-status-posthog

deployment-status-posthog Bot commented Jul 15, 2026

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-07-15 13:04 UTC Run
prod-us ✅ Deployed 2026-07-15 13:30 UTC Run
prod-eu ✅ Deployed 2026-07-15 13:30 UTC Run

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.

2 participants