Skip to content

pam/adapter: skip authModeSelection stage for interactive terminal#1524

Open
adombeck wants to merge 1 commit into
mainfrom
1523-fix-broker-selection
Open

pam/adapter: skip authModeSelection stage for interactive terminal#1524
adombeck wants to merge 1 commit into
mainfrom
1523-fix-broker-selection

Conversation

@adombeck

@adombeck adombeck commented May 19, 2026

Copy link
Copy Markdown
Contributor

When authenticating via an interactive terminal (e.g. sudo), the auth
mode selection list was often briefly visible before the first mode was
auto-selected and the challenge view appeared.

Fix this by skipping the transition to the authModeSelection stage
entirely for InteractiveTerminal clients: fetch auth modes and
immediately auto-select the first one.

For GDM and Native clients the existing behaviour is preserved:

  • GDM fetches modes then transitions to authModeSelection, deferring
    the auto-selection until the list is focused.
  • Native MFA (auth.Next) transitions to authModeSelection first so that
    when modes arrive the list is already focused and the immediate
    auto-selection safely drives the next challenge.

@3v1n0

3v1n0 commented May 19, 2026

Copy link
Copy Markdown
Contributor

I feel this opens the gate for races, I did have another branch that was avoiding showing it but it was instead waiting and using spinners in between stages.

I need to find out in my stashes :), but it was avoiding the re-introduction of races.

@3v1n0

3v1n0 commented May 19, 2026

Copy link
Copy Markdown
Contributor

The rationale for this was explained in 52abc8f

@adombeck

Copy link
Copy Markdown
Contributor Author

The rationale for this was explained in 52abc8f

Yeah I've seen that and still plan to look into that to decide if/how to continue here

@adombeck adombeck force-pushed the 1523-fix-broker-selection branch 3 times, most recently from 220c7fe to 9b45a46 Compare May 20, 2026 17:33

Copilot AI commented May 20, 2026

Copy link
Copy Markdown
Contributor

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • dl.google.com
    • Triggering command: /usr/lib/apt/methods/https /usr/lib/apt/methods/https -slog -stringintconv -tests (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

When authenticating via an interactive terminal (e.g. sudo), the auth
mode selection list was often briefly visible before the first mode was
auto-selected and the challenge view appeared.

Fix this by skipping the transition to the authModeSelection stage
entirely for InteractiveTerminal clients: fetch auth modes and
immediately auto-select the first one.

For GDM and Native clients the existing behaviour is preserved:
- GDM fetches modes then transitions to authModeSelection, deferring
  the auto-selection until the list is focused.
- Native MFA (auth.Next) transitions to authModeSelection first so that
  when modes arrive the list is already focused and the immediate
  auto-selection safely drives the next challenge.
@adombeck adombeck force-pushed the 1523-fix-broker-selection branch from 3049037 to 2852bc7 Compare May 21, 2026 07:47
@codecov

codecov Bot commented May 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.02%. Comparing base (936e4ce) to head (2852bc7).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1524      +/-   ##
==========================================
- Coverage   87.02%   87.02%   -0.01%     
==========================================
  Files          93       93              
  Lines        6443     6449       +6     
  Branches      111      111              
==========================================
+ Hits         5607     5612       +5     
- Misses        780      781       +1     
  Partials       56       56              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@adombeck adombeck changed the title pam/adapter: Fix broker selection shown before auto-selection pam/adapter: skip authModeSelection stage for interactive terminal May 21, 2026
@adombeck

Copy link
Copy Markdown
Contributor Author

@3v1n0 I think the current change should avoid races but still avoid briefly showing the selection. Please take a look :)

@adombeck adombeck requested a review from 3v1n0 May 21, 2026 08:15
@adombeck adombeck marked this pull request as ready for review May 21, 2026 08:15

@denisonbarbosa denisonbarbosa left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

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.

4 participants