From dfe3208c82f828d4c79361fcf15b1bda309273be Mon Sep 17 00:00:00 2001 From: Alexander Herold Date: Fri, 19 Jun 2026 19:54:17 +0200 Subject: [PATCH] Auth: fix OAuth online-status send deadlocking the control loop (#31036) Co-authored-by: Alexander Herold --- plugin/auth/demo.go | 21 ++++++++++++++------- plugin/auth/oauth.go | 17 +++++++++++++---- plugin/auth/oauth_test.go | 21 +++++++++++++++++++++ server/providerauth/providerauth.go | 4 +++- 4 files changed, 51 insertions(+), 12 deletions(-) diff --git a/plugin/auth/demo.go b/plugin/auth/demo.go index 66c719c27..446187a11 100644 --- a/plugin/auth/demo.go +++ b/plugin/auth/demo.go @@ -71,11 +71,22 @@ func NewDemo(server, method, redirectUri, secret string) (oauth2.TokenSource, er demoInstance.onlineC = onlineC // Send initial auth status - demoInstance.onlineC <- false + demoInstance.setOnline(false) return demoInstance, nil } +// setOnline notifies the auth handler without blocking; see OAuth.setOnline. +func (o *demo) setOnline(online bool) { + if o.onlineC == nil { + return + } + select { + case o.onlineC <- online: + default: + } +} + func (o *demo) Token() (*oauth2.Token, error) { if o.token == nil { return nil, api.LoginRequiredError("demo") @@ -116,9 +127,7 @@ func (o *demo) Login(state string) (string, *oauth2.DeviceAuthResponse, error) { func (o *demo) Logout() error { o.token = nil - if o.onlineC != nil { - o.onlineC <- false - } + o.setOnline(false) return nil } @@ -136,9 +145,7 @@ func (o *demo) HandleCallback(params url.Values) error { } // Notify that authentication succeeded - if o.onlineC != nil { - o.onlineC <- true - } + o.setOnline(true) return nil } diff --git a/plugin/auth/oauth.go b/plugin/auth/oauth.go index e94606a39..44ee17367 100644 --- a/plugin/auth/oauth.go +++ b/plugin/auth/oauth.go @@ -146,7 +146,7 @@ func NewOAuth(ctx context.Context, name, device string, oc *oauth2.Config, opts } o.onlineC = onlineC - o.onlineC <- token.Valid() + o.setOnline(token.Valid()) // add instance addInstance(o.subject, o) @@ -172,7 +172,7 @@ func (o *OAuth) Token() (*oauth2.Token, error) { // force logout if strings.Contains(err.Error(), "invalid_") && settings.Exists(o.subject) { o.token = nil - o.onlineC <- false + o.setOnline(false) settings.Delete(o.subject) } @@ -199,7 +199,16 @@ func (o *OAuth) updateToken(token *oauth2.Token) { o.token = token - o.onlineC <- token.Valid() + o.setOnline(token.Valid()) +} + +// setOnline signals the auth handler without blocking; the value is only a +// wakeup. A blocking send under o.mu would deadlock via Authenticated()->Token(). +func (o *OAuth) setOnline(online bool) { + select { + case o.onlineC <- online: + default: + } } // HandleCallback implements api.AuthProvider. @@ -272,7 +281,7 @@ func (o *OAuth) Logout() error { defer o.mu.Unlock() o.token = nil - o.onlineC <- false + o.setOnline(false) return nil } diff --git a/plugin/auth/oauth_test.go b/plugin/auth/oauth_test.go index 401ea9e1a..09799e360 100644 --- a/plugin/auth/oauth_test.go +++ b/plugin/auth/oauth_test.go @@ -2,6 +2,7 @@ package auth import ( "testing" + "time" "github.com/stretchr/testify/require" "golang.org/x/oauth2" @@ -32,3 +33,23 @@ func TestOAuth(t *testing.T) { require.True(t, token.Valid()) require.Equal(t, 1, storerCalled) } + +// TestSetOnlineNonBlocking ensures setOnline coalesces instead of blocking when +// the channel isn't drained, guarding the token-refresh deadlock. +func TestSetOnlineNonBlocking(t *testing.T) { + o := &OAuth{onlineC: make(chan bool, 1)} + o.setOnline(true) // fill the buffer; nobody is draining it + + done := make(chan struct{}) + go func() { + o.setOnline(false) // would block forever on a full unbuffered/direct send + o.setOnline(true) + close(done) + }() + + select { + case <-done: + case <-time.After(time.Second): + t.Fatal("setOnline blocked while the online channel was not drained") + } +} diff --git a/server/providerauth/providerauth.go b/server/providerauth/providerauth.go index 7dc22b435..3330f8fee 100644 --- a/server/providerauth/providerauth.go +++ b/server/providerauth/providerauth.go @@ -52,7 +52,9 @@ func Register(name string, handler api.AuthProvider) (chan<- bool, error) { return nil, err } - onlineC := make(chan bool) + // buffered + non-blocking send (see OAuth.setOnline): the value is only a + // signal and the handler re-reads live state, so coalescing is lossless. + onlineC := make(chan bool, 1) go func() { for range onlineC {