mirror of
https://github.com/netbirdio/netbird.git
synced 2026-07-17 12:09:58 +00:00
[management] detach JWT group sync write from request cancellation (#6621)
The HTTP auth middleware runs syncUserJWTGroups in the request context. The dashboard SPA routinely aborts in-flight requests on re-render or navigation, which cancels the request context mid-write and rolls back the group-sync DB transaction. The error is logged but swallowed, so the synced groups silently never persist (users.auto_groups stays empty) while the failing log line repeats on every request. Detach the sync from the request's cancellation with context.WithoutCancel so the write can commit regardless of the client connection; the store already bounds the transaction with its own timeout. Add a regression test asserting the sync receives a non-cancelled context even when the originating request is cancelled.
This commit is contained in:
@@ -152,7 +152,11 @@ func (m *AuthMiddleware) checkJWTFromRequest(r *http.Request, authHeaderParts []
|
||||
return err
|
||||
}
|
||||
|
||||
err = m.syncUserJWTGroups(ctx, userAuth)
|
||||
// Detach the group-sync write from the request's cancellation: the dashboard
|
||||
// SPA aborts in-flight requests on re-render, which would otherwise cancel the
|
||||
// DB transaction mid-write and silently drop the synced groups. Context values
|
||||
// (request id, logger) are preserved; the store bounds the tx with its own timeout.
|
||||
err = m.syncUserJWTGroups(context.WithoutCancel(ctx), userAuth)
|
||||
if err != nil {
|
||||
log.WithContext(ctx).Errorf("HTTP server failed to sync user JWT groups: %s", err)
|
||||
}
|
||||
|
||||
@@ -241,6 +241,66 @@ func TestAuthMiddleware_Handler(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestAuthMiddleware_SyncUserJWTGroupsDetachedFromRequestCancellation ensures the
|
||||
// JWT group sync write is not bound to the request context. The dashboard SPA
|
||||
// routinely aborts in-flight requests on re-render/navigation; if the sync ran in
|
||||
// the request context, the cancellation would roll back the DB transaction and the
|
||||
// synced groups would silently never persist. The sync must receive a context that
|
||||
// is not cancelled even when the originating request is.
|
||||
func TestAuthMiddleware_SyncUserJWTGroupsDetachedFromRequestCancellation(t *testing.T) {
|
||||
var (
|
||||
syncCalled bool
|
||||
syncCtxErr error
|
||||
)
|
||||
|
||||
mockAuth := &auth.MockManager{
|
||||
ValidateAndParseTokenFunc: mockValidateAndParseToken,
|
||||
EnsureUserAccessByJWTGroupsFunc: mockEnsureUserAccessByJWTGroups,
|
||||
MarkPATUsedFunc: mockMarkPATUsed,
|
||||
GetPATInfoFunc: mockGetAccountInfoFromPAT,
|
||||
}
|
||||
|
||||
disabledLimiter := NewAPIRateLimiter(nil)
|
||||
disabledLimiter.SetEnabled(false)
|
||||
|
||||
authMiddleware := NewAuthMiddleware(
|
||||
mockAuth,
|
||||
func(ctx context.Context, userAuth nbauth.UserAuth) (string, string, error) {
|
||||
return userAuth.AccountId, userAuth.UserId, nil
|
||||
},
|
||||
func(ctx context.Context, userAuth nbauth.UserAuth) error {
|
||||
syncCalled = true
|
||||
syncCtxErr = ctx.Err()
|
||||
return nil
|
||||
},
|
||||
func(ctx context.Context, userAuth nbauth.UserAuth) (*types.User, error) {
|
||||
return &types.User{}, nil
|
||||
},
|
||||
disabledLimiter,
|
||||
nil,
|
||||
func(_ context.Context, _, _, _ string) bool { return false },
|
||||
)
|
||||
|
||||
handlerToTest := authMiddleware.Handler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
|
||||
|
||||
// Simulate the dashboard aborting the request: it arrives already cancelled.
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
cancel()
|
||||
|
||||
req := httptest.NewRequest("GET", "http://testing/test", nil).WithContext(ctx)
|
||||
req.Header.Set("Authorization", "Bearer "+JWT)
|
||||
rec := httptest.NewRecorder()
|
||||
|
||||
handlerToTest.ServeHTTP(rec, req)
|
||||
|
||||
if !syncCalled {
|
||||
t.Fatal("syncUserJWTGroups was not called")
|
||||
}
|
||||
if syncCtxErr != nil {
|
||||
t.Fatalf("syncUserJWTGroups received a cancelled context (%v); the group-sync write must be detached from request cancellation", syncCtxErr)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAuthMiddleware_RateLimiting(t *testing.T) {
|
||||
mockAuth := &auth.MockManager{
|
||||
ValidateAndParseTokenFunc: mockValidateAndParseToken,
|
||||
|
||||
Reference in New Issue
Block a user