[PR #6621] [MERGED] [management] detach JWT group sync write from request cancellation #28610

Open
opened 2026-08-05 08:06:38 -04:00 by saavagebueno · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/netbirdio/netbird/pull/6621
Author: @den-dw
Created: 6/30/2026
Status: Merged
Merged: 7/1/2026
Merged by: @pascal-fischer

Base: mainHead: fix/jwt-group-sync-request-cancel


📝 Commits (1)

  • 305f9a3 [management] detach JWT group sync write from request cancellation

📊 Changes

2 files changed (+65 additions, -1 deletions)

View changed files

📝 management/server/http/middleware/auth_middleware.go (+5 -1)
📝 management/server/http/middleware/auth_middleware_test.go (+60 -0)

📄 Description

Describe your changes

The HTTP auth middleware persists a user's JWT-derived groups (users.auto_groups) by calling syncUserJWTGroups inside the request context. The dashboard SPA routinely aborts in-flight fetch() calls on re-render/navigation, which cancels the request context mid-write. The group-sync write runs in a DB transaction bound to that context, so it is rolled back and the synced groups never persist. The error is logged but swallowed (no return), so it repeats silently on every request and auto_groups stays empty.

Root cause: a server-side persistence side effect was tied to the client request lifecycle.

Fix: detach the group-sync write with context.WithoutCancel(ctx) so it commits regardless of the client connection, while preserving context values (request id, logger). This matches the existing context.WithoutCancel usage in the management server (peer.go, the gRPC server, the network-map controller). The transaction stays bounded by the store's own timeout (ExecuteInTransaction, default 5 min).

Scoped to the HTTP middleware call-site where the SPA-abort symptom reproduces. The gRPC login path (internals/shared/grpc/server.go) calls the same method but isn't driven by an aborting SPA and is intentionally left out of this change.

A regression test (TestAuthMiddleware_SyncUserJWTGroupsDetachedFromRequestCancellation) dispatches an already-cancelled request and asserts the injected syncUserJWTGroups receives a non-cancelled context — it fails without the change (context canceled) and passes with it.

Fixes #6620

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

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 server bug fix in the auth middleware, no user-facing surface.

🔄 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/6621 **Author:** [@den-dw](https://github.com/den-dw) **Created:** 6/30/2026 **Status:** ✅ Merged **Merged:** 7/1/2026 **Merged by:** [@pascal-fischer](https://github.com/pascal-fischer) **Base:** `main` ← **Head:** `fix/jwt-group-sync-request-cancel` --- ### 📝 Commits (1) - [`305f9a3`](https://github.com/netbirdio/netbird/commit/305f9a35b8655447447ad1d237fd2fce69387d1f) [management] detach JWT group sync write from request cancellation ### 📊 Changes **2 files changed** (+65 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `management/server/http/middleware/auth_middleware.go` (+5 -1) 📝 `management/server/http/middleware/auth_middleware_test.go` (+60 -0) </details> ### 📄 Description ## Describe your changes The HTTP auth middleware persists a user's JWT-derived groups (`users.auto_groups`) by calling `syncUserJWTGroups` inside the **request context**. The dashboard SPA routinely aborts in-flight `fetch()` calls on re-render/navigation, which cancels the request context mid-write. The group-sync write runs in a DB transaction bound to that context, so it is rolled back and the synced groups never persist. The error is logged but swallowed (no `return`), so it repeats silently on every request and `auto_groups` stays empty. **Root cause:** a server-side persistence side effect was tied to the client request lifecycle. **Fix:** detach the group-sync write with `context.WithoutCancel(ctx)` so it commits regardless of the client connection, while preserving context values (request id, logger). This matches the existing `context.WithoutCancel` usage in the management server (`peer.go`, the gRPC server, the network-map controller). The transaction stays bounded by the store's own timeout (`ExecuteInTransaction`, default 5 min). Scoped to the HTTP middleware call-site where the SPA-abort symptom reproduces. The gRPC login path (`internals/shared/grpc/server.go`) calls the same method but isn't driven by an aborting SPA and is intentionally left out of this change. A regression test (`TestAuthMiddleware_SyncUserJWTGroupsDetachedFromRequestCancellation`) dispatches an already-cancelled request and asserts the injected `syncUserJWTGroups` receives a non-cancelled context — it fails without the change (`context canceled`) and passes with it. ## Issue ticket number and link Fixes #6620 ### 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) - [x] This change does **not** modify the public API, gRPC protocols, functionality behavior, CLI / service flags, or introduce a new feature > 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 server bug fix in the auth middleware, no user-facing surface. --- <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:06:39 -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#28610