Auth: fix OAuth online-status send deadlocking the control loop (#31036)
Co-authored-by: Alexander Herold <alexander@herold.hn>
This commit is contained in:
parent
72d6e65b0b
commit
dfe3208c82
4 changed files with 51 additions and 12 deletions
|
|
@ -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
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue