pam/adapter: Ignore stale stopAuthentication from a superseded challenge#1626
Draft
adombeck wants to merge 2 commits into
Draft
pam/adapter: Ignore stale stopAuthentication from a superseded challenge#1626adombeck wants to merge 2 commits into
adombeck wants to merge 2 commits into
Conversation
Device authentication through GDM could hang on the QR screen even after the user completed the browser flow. The QR screen showed one device code while the broker kept polling for a different one, so the authorization the user granted was never observed and the login stalled until the device code expired. When an auth mode is auto-selected, the adapter both calls SelectAuthenticationMode (minting a device code and starting the poll) and tells GDM to select that mode. GDM echoes the selection back in its next poll, and the adapter treated that echo as a fresh selection, issuing a second SelectAuthenticationMode that minted a new device code and orphaned the first poll. Track the selection we send to GDM and drop its echo. The suppression is a one-shot consumed by the first matching echo, and is also cleared on any stage change, so a genuine re-selection of the same mode (the user navigating back to auth mode selection and picking it again) is still honored. A pure stage-change reset raced with the echo in the change-password flow; a pure one-shot swallowed legitimate re-selection of an auto-selected single mode. Combining both covers each gap. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
After successful device authentication through GDM, the broker returns "next" and authd switches to the newpassword challenge so the user can set a local password. The "Create a local password" entry would sometimes not appear, leaving the login stuck on the greeter until the device code expired (intermittent; see #1424, #1414). Switching auth modes tears down the current challenge via authenticationModel.Reset(), which schedules cancelIsAuthenticated() — an asynchronous command that emits stopAuthentication once the in-flight IsAuthenticated call has been cancelled. The new challenge is composed and started in the meantime. When the stale stopAuthentication from the previous challenge's cancellation arrived after the new challenge had started, it cleared inProgress and the entry was never rendered. Stamp each stopAuthentication with the challenge generation current when it was scheduled, bump the generation when a new challenge is composed, and ignore a stop whose generation no longer matches. A stop belonging to a superseded challenge can no longer tear down the current one. The exact-event-count assertions added alongside the GDM echo suppression are removed from TestGdmModel: they race with the test conversation handler, which delivers echo and stage-change events concurrently with the model's polls. The invariant they checked (the echo of a selection must not add an extra selection cycle) is covered deterministically in gdmmodel_authmode_echo_test.go. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
adombeck
added
the
e2e-tests
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1626 +/- ##
==========================================
+ Coverage 84.24% 88.00% +3.75%
==========================================
Files 21 99 +78
Lines 1168 6843 +5675
Branches 0 111 +111
==========================================
+ Hits 984 6022 +5038
- Misses 184 765 +581
- Partials 0 56 +56 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
3v1n0
reviewed
Jun 25, 2026
| } | ||
|
|
||
| func (m gdmModel) handlePollResponse(gdmPollResults []*gdm.EventData) tea.Cmd { | ||
| func (m *gdmModel) handlePollResponse(gdmPollResults []*gdm.EventData) tea.Cmd { |
Contributor
There was a problem hiding this comment.
The change here goes against the point of using returned copy of the model itself to do changes, in fact ideally no method should use a pointer receiver.
adombeck
changed the title
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
2 participants
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.
After successful device authentication through GDM, the broker returns "next" and authd switches to the newpassword challenge so the user can set a local password. The "Create a local password" entry would sometimes not appear, leaving the login stuck on the greeter until the device code expired (intermittent; see #1424, #1414).
Switching auth modes tears down the current challenge via authenticationModel.Reset(), which schedules cancelIsAuthenticated() — an asynchronous command that emits stopAuthentication once the in-flight IsAuthenticated call has been cancelled. The new challenge is composed and started in the meantime. When the stale stopAuthentication from the previous challenge's cancellation arrived after the new challenge had started, it cleared inProgress and the entry was never rendered.
Stamp each stopAuthentication with the challenge generation current when it was scheduled, bump the generation when a new challenge is composed, and ignore a stop whose generation no longer matches. A stop belonging to a superseded challenge can no longer tear down the current one.
The exact-event-count assertions added alongside the GDM echo suppression are removed from TestGdmModel: they race with the test conversation handler, which delivers echo and stage-change events concurrently with the model's polls. The invariant they checked (the echo of a selection must not add an extra selection cycle) is covered deterministically in gdmmodel_authmode_echo_test.go.