Skip to content

fix(login): wait for either 2FA input target to avoid SPA re-render race#283

Closed
Srain021 wants to merge 3 commits intoVoyz:masterfrom
Srain021:pr/step-two-fa-race-fix
Closed

fix(login): wait for either 2FA input target to avoid SPA re-render race#283
Srain021 wants to merge 3 commits intoVoyz:masterfrom
Srain021:pr/step-two-fa-race-fix

Conversation

@Srain021
Copy link
Copy Markdown

Problem

In the Mobile Authenticator App flow (on the second-factor-selection branch from #277), IBeam can time out after generating a TOTP code even though the input is visibly present on the page.

Root cause: step_two_fa calls _find_two_fa_input_target(targets, driver) synchronously right after the TOTP code is generated. That helper does a point-in-time DOM lookup without waiting. After step_select_two_fa selects a method from the dropdown, the IBKR SPA briefly re-renders, which removes input[placeholder*="Code"] from the DOM for a short window. If the snapshot lands inside that window, the lookup falls back to TWO_FA_INPUT (ID@@xyz-field-bronze-response), which does not exist in the Mobile Authenticator App flow, and the subsequent wait_and_identify_trigger never finds the element and times out after 60s — even though TOTP was generated correctly.

Fix

Replace the eager snapshot + lookup with a direct wait_and_identify_trigger that accepts either TWO_FA_INPUT or TWO_FA_INPUT_GENERIC. Selenium then natively waits for whichever input is active for the flow, removing the race without changing selector semantics.

Testing

Tested on aarch64 Linux running the second-factor-selection branch from #277:

  • IBEAM_TWO_FA_SELECT=True + IBEAM_TWO_FA_SELECT_TARGET="Mobile Authenticator App" dropdown selection
  • TOTP auto-entry via a custom handler
  • Authentication now succeeds consistently (previously flaky due to this race)
  • Session persists across container restarts

Depends on

This patch applies on top of #277, because TWO_FA_INPUT_GENERIC is introduced by that PR. The branch is based on onurserce's master (same base as #277) to make the dependency explicit. If #277 is merged first, this diff cleanly applies to Voyz/ibeam:master.

Open Questions

  1. Would you prefer the fix inside _find_two_fa_input_target (keeping that helper) instead of changing the call site? Happy to refactor either way.
  2. Should I add a DOM-unit regression test around this SPA re-render timing, or is that not realistic for this login flow?

onurserce and others added 3 commits April 21, 2026 10:36
When step_select_two_fa selects a 2FA method from the dropdown, IBKR's
login SPA briefly re-renders, which can remove input[placeholder*="Code"]
from the DOM for a short window. The current step_two_fa path calls
_find_two_fa_input_target() synchronously right after the TOTP code is
generated, and that call does a point-in-time DOM snapshot without waiting.
If the snapshot lands inside the re-render gap, the lookup falls back to
TWO_FA_INPUT (xyz-field-bronze-response), which does not exist in the
Mobile Authenticator App flow, and the subsequent wait_and_identify_trigger
times out after 60s even though TOTP was generated correctly.

Fix: replace the eager snapshot + lookup with a direct
wait_and_identify_trigger on both TWO_FA_INPUT and TWO_FA_INPUT_GENERIC,
so Selenium natively waits for whichever input is active for the flow.
This removes the race without changing the selector semantics.

Tested on aarch64 Linux running this fork's second-factor-selection
branch: TOTP auto-entry completed on the Mobile Authenticator App flow
and the session persisted across container restarts.
@Srain021 Srain021 force-pushed the pr/step-two-fa-race-fix branch from 6fc1f54 to 2bc8b5d Compare April 21, 2026 02:37
@Voyz
Copy link
Copy Markdown
Owner

Voyz commented Apr 21, 2026

Hi @Srain021 thanks for spotting this issue and submitting it. Do I understand correctly this is an extension of #277 ? If so, I think it would be reasonable create a patch to https://github.com/onurserce/ibeam/tree/master branch, not to IBeam master - so that @onurserce's contributions are preserved.

If I'm misreading this, please ellaborate.

In either case, your observation seems valid and the fix should be included, good job bringing this up - thanks! 🙌

@Srain021
Copy link
Copy Markdown
Author

Sorry for the back-and-forth @Voyz — this is my first open-source PR and I fumbled the close/reopen flow earlier (accidentally reopened this one when I was trying to recover from another mis-click). As you originally suggested, the fix belongs against onurserce's fork since the bug is in #277's code, so it's now at onurserce#1. Closing this for good — thanks for your patience while I figure GitHub out!

@Srain021 Srain021 closed this Apr 25, 2026
@Voyz
Copy link
Copy Markdown
Owner

Voyz commented Apr 25, 2026

Mate don't worry at all 😊 Thanks for clarifying this and being transparent 👍 Hope the other PR gets merged

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.

3 participants