[PR #6983] [MERGED] [client] Don't ask for an SSO login when the login never reached management #29360

Closed
opened 2026-08-05 08:07:55 -04:00 by saavagebueno · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/netbirdio/netbird/pull/6983
Author: @lixmal
Created: 7/30/2026
Status: Merged
Merged: 8/3/2026
Merged by: @lixmal

Base: mainHead: worktree-daemon-login-transport-error


📝 Commits (2)

  • 48578b8 Return the login failure instead of demanding SSO when management is unreachable
  • 6789014 Assert the login status after the SSO flow starts

📊 Changes

2 files changed (+122 additions, -4 deletions)

View changed files

client/server/login_outcome_test.go (+89 -0)
📝 client/server/server.go (+33 -4)

📄 Description

Describe your changes

The daemon treated every failed login attempt as "this peer needs to log in", so a login that never reached the management server (unreachable, restarting, dial timeout) surfaced as an SSO prompt with a device code instead of a retryable error. A netbird up that happened to run while management was briefly away asked the user to re-authenticate, even though the session was still valid.

  • Only start the SSO flow when management actually refused the peer's credentials; return any other login failure so the CLI's existing backoff can retry it
  • Keep the login outcome that was already classified by the login attempt, matching how the setup-key and JWT paths already handle it

Stack

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)
  • This change does not modify the public API, gRPC protocols, functionality behavior, CLI / service flags, or introduce a new feature — OR I have discussed it with the NetBird team beforehand (link the issue / Slack thread in the description). See CONTRIBUTING.md.

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change (explain why): internal error handling in the daemon, no user-facing surface changes.

Docs PR URL (required if "docs added" is checked)

Paste the PR link from https://github.com/netbirdio/docs here:

https://github.com/netbirdio/docs/pull/__

Summary by CodeRabbit

  • Bug Fixes
    • Improved login handling when Management is unreachable by returning a retryable error and preserving the failed-login state.
    • Login now starts the SSO flow only when it’s actually required, avoiding incorrect SSO triggers on other error types.
    • Permission-denied during the auth check now correctly proceeds into the SSO sign-in flow.
  • Tests
    • Added coverage for Management connectivity failures, auth refusal behavior, and SSO login transition outcomes.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/netbirdio/netbird/pull/6983 **Author:** [@lixmal](https://github.com/lixmal) **Created:** 7/30/2026 **Status:** ✅ Merged **Merged:** 8/3/2026 **Merged by:** [@lixmal](https://github.com/lixmal) **Base:** `main` ← **Head:** `worktree-daemon-login-transport-error` --- ### 📝 Commits (2) - [`48578b8`](https://github.com/netbirdio/netbird/commit/48578b8fae4dba5c56e93dd323788144fde5a4c5) Return the login failure instead of demanding SSO when management is unreachable - [`6789014`](https://github.com/netbirdio/netbird/commit/67890146b6aa3781939927cb13fdcaa281260540) Assert the login status after the SSO flow starts ### 📊 Changes **2 files changed** (+122 additions, -4 deletions) <details> <summary>View changed files</summary> ➕ `client/server/login_outcome_test.go` (+89 -0) 📝 `client/server/server.go` (+33 -4) </details> ### 📄 Description ## Describe your changes The daemon treated every failed login attempt as "this peer needs to log in", so a login that never reached the management server (unreachable, restarting, dial timeout) surfaced as an SSO prompt with a device code instead of a retryable error. A `netbird up` that happened to run while management was briefly away asked the user to re-authenticate, even though the session was still valid. - Only start the SSO flow when management actually refused the peer's credentials; return any other login failure so the CLI's existing backoff can retry it - Keep the login outcome that was already classified by the login attempt, matching how the setup-key and JWT paths already handle it ## Issue ticket number and link ## Stack <!-- branch-stack --> ### Checklist - [x] Is it a bug fix - [ ] Is a typo/documentation fix - [ ] Is a feature enhancement - [ ] It is a refactor - [x] Created tests that fail without the change (if possible) - [ ] This change does **not** modify the public API, gRPC protocols, functionality behavior, CLI / service flags, or introduce a new feature — **OR** I have discussed it with the NetBird team beforehand (link the issue / Slack thread in the description). See [CONTRIBUTING.md](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTING.md#discuss-changes-with-the-netbird-team-first). > By submitting this pull request, you confirm that you have read and agree to the terms of the [Contributor License Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md). ## Documentation Select exactly one: - [ ] I added/updated documentation for this change - [x] Documentation is **not needed** for this change (explain why): internal error handling in the daemon, no user-facing surface changes. ### Docs PR URL (required if "docs added" is checked) Paste the PR link from https://github.com/netbirdio/docs here: https://github.com/netbirdio/docs/pull/__ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved login handling when Management is unreachable by returning a retryable error and preserving the failed-login state. - Login now starts the SSO flow only when it’s actually required, avoiding incorrect SSO triggers on other error types. - Permission-denied during the auth check now correctly proceeds into the SSO sign-in flow. - **Tests** - Added coverage for Management connectivity failures, auth refusal behavior, and SSO login transition outcomes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
saavagebueno added the pull-request label 2026-08-05 08:07:56 -04:00
Sign in to join this conversation.
No Label pull-request
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: DYNR/netbird#29360