fix(relay): keep sign-in polling idempotent once the authority redeemed a transaction - #3248
Closed
braindeadz wants to merge 1 commit into
Closed
braindeadz wants to merge 1 commit into
braindeadz wants to merge 1 commit into
Conversation
…ed a transaction The identity authority redeems a sign-in transaction exactly once: the first poll that finds it completed returns the tokens, and every later poll for the same transaction answers `consumed` without them. Both native clients poll on an interval, so a response that is lost, a request repeated after a timeout, or a process that resumes after the browser handoff redeems the grant without the client ever seeing the tokens. That sign-in can then never complete: the app polls until the window closes and reports a generic failure, which is what GCWing#3246 reports (browser says "login complete", phone says the relay returned an invalid account response). Replay the terminal answer this relay has already received, keyed by transaction id and validated against the same transaction secret, so a repeated poll sees what the first poll saw. Pending answers are still asked upstream, and entries expire with the transaction window. A poll presenting a different secret never receives the stored payload. Fixes GCWing#3246
Author
|
CI note: the two workflow runs on this branch are sitting in If CI reports a compile error in the added test helper, I will fix it promptly; the production-facing part of the change is confined to |
Collaborator
|
Superseded by #3253. The replacement keeps the idempotent polling fix, scopes replay by transaction_id and transaction_secret, adds regression coverage, and includes the Android/HarmonyOS secure-store recovery fixes. |
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.
Summary
Makes the relay's sign-in poll idempotent: once the identity authority has answered a transaction, the relay replays that answer instead of asking again, because the authority's grant is single-use.
Fixes #3246
Type and Areas
Type: bug fix (regression fix)
Areas: server/relay (
src/crates/services/relay-service/src/identity.rs)Motivation / Impact
Measured on production (
https://remote.openbitfun.com/v/1.0.2), same transaction, two consecutive polls with the sametransactionId/transactionSecret:Both native clients poll on an interval (
AuthorizationPoll.awaitAccessToken, everypollIntervalSeconds, plus one poll immediately after the browser handoff), and only understandauthorized/expired/denied. So a poll whose response is lost, a request repeated after a timeout, or a process resumed after the handoff redeems the grant without the client ever seeing the tokens; from then on the sign-in can never complete and the app ends on a generic failure. That matches #3246 exactly: the browser reports "login complete", the phone reports "the relay returned an invalid account response".Impact: a repeated poll now returns the payload the first poll received, for the length of the transaction window, so an already-installed APK completes a sign-in it would previously lose.
pendinganswers are still forwarded upstream, and a poll presenting a different secret is refused rather than handed the stored payload.Verification
Not compiled locally: I have no Rust toolchain on this machine, so
cargo test -p openbitfun-relay-service identitywas not run and this change relies on your CI. Please treat the patch as needing a compile pass before merge.What was verified:
auth/desktop/start->auth/email/send->auth/email/verify-> twopollcalls ->login), which is what the patch addresses.poll_replays_a_completed_transaction_instead_of_redeeming_it_twiceis written to fail on the pre-patch code: it asserts the authority is called once for a repeated poll, that both payloads are identical, and that a mismatching secret seesconsumedinstead of the issued token.device_kind: "mobile",clientVersion,clientProtocol), andloginanswers 200 with them.Reviewer Notes
POLL_REPLAY_SECS(900 s), trimmed on every access. A relay behind multiple instances would still need the authority itself to be idempotent; this patch removes the failure for a single relay (as deployed today).std::sync::Mutexis deliberate: every guard is dropped before anyawait, so no lock is held across a suspension point.Checklist