Skip to content

pam/main-exec: Don't set timeout for D-Bus connection#1448

Merged
adombeck merged 2 commits intomainfrom
1431-fix-pam-dbus-timeout
Apr 16, 2026
Merged

pam/main-exec: Don't set timeout for D-Bus connection#1448
adombeck merged 2 commits intomainfrom
1431-fix-pam-dbus-timeout

Conversation

@adombeck
Copy link
Copy Markdown
Contributor

@adombeck adombeck commented Apr 14, 2026

We've seen cases in e2e-tests where device auth took longer than 2 minutes to complete. When it finally succeeds, the login fails because the subsequent SetData(authenticationBrokerIDKey, ...) call fails:

failed to call com.ubuntu.authd.pam.SetData: dbus: connection closed by user LOGIN: exiting with error System error: dbus: connection closed by user

We don't to enforce a timeout for the login procedure here. If there should be a timeout, it should be the caller who enforces it - similar to the behavior of loginwhich is configured via LOGIN_TIMEOUT in /etc/login.defs.

Closes #1431
UDENG-9792

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 14, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.06%. Comparing base (15c4664) to head (56fcbd0).
⚠️ Report is 27 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1448      +/-   ##
==========================================
+ Coverage   87.05%   87.06%   +0.01%     
==========================================
  Files          93       93              
  Lines        6367     6365       -2     
  Branches      111      111              
==========================================
- Hits         5543     5542       -1     
+ Misses        768      767       -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.

@3v1n0
Copy link
Copy Markdown
Contributor

3v1n0 commented Apr 15, 2026

Maybe we can override the default with an env variable for tests?

@adombeck
Copy link
Copy Markdown
Contributor Author

Maybe we can override the default with an env variable for tests?

I don't see this is as a test only problem (I should probably make that clear in the commit message as well). I can also see how users performing device auth with MFA could easily take longer than 2 minutes to complete the login.

I was wondering why we have this timeout in the first place. It was added by commit aa48751 which says "so that in case of problems we don't hang indefinitely". Should it maybe be the caller's responsibility to cancel the operation when it thinks it's taking too long - similar to the behavior of login which is configured via LOGIN_TIMEOUT in /etc/login.defs?

@3v1n0
Copy link
Copy Markdown
Contributor

3v1n0 commented Apr 15, 2026

Oh wait, this was meant to be the timeout to init the connection.

If it's not, then we should definely not have a timeout at all that controls how long the user can stay logging (or maybe just follow the system settings as said).

The point here was to wait few seconds before we had a connection with the bus only.

We've seen cases in e2e-tests where device auth took longer than 2
minutes to complete. When it finally succeeds, the login fails because
the subsequent SetData(authenticationBrokerIDKey, ...) call fails:

  failed to call com.ubuntu.authd.pam.SetData: dbus: connection closed by user
  LOGIN: exiting with error System error: dbus: connection closed by user

We don't to enforce a timeout for the login procedure here. If there
should be a timeout, it should be the caller who enforces it - similar
to the behavior of `login`which is configured via LOGIN_TIMEOUT in
/etc/login.defs.
@adombeck adombeck force-pushed the 1431-fix-pam-dbus-timeout branch from 68be732 to cab60c2 Compare April 15, 2026 11:05
@adombeck adombeck changed the title pam/main-exec: Increase timeout for D-Bus connection pam/main-exec: Don't set timeout for D-Bus connection Apr 15, 2026
@adombeck
Copy link
Copy Markdown
Contributor Author

If it's not, then we should definely not have a timeout at all

I removed it

@adombeck adombeck marked this pull request as ready for review April 15, 2026 22:33
@adombeck adombeck merged commit 9a73399 into main Apr 16, 2026
36 of 43 checks passed
@adombeck adombeck deleted the 1431-fix-pam-dbus-timeout branch April 16, 2026 07:06
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.

CLI login sometimes broken after successful device auth

3 participants