Skip to content

Commit d060e6e

Browse files
authored
Merge pull request #738 from openziti/add-auth-lock
Add back locking around authenticate. Fix deadlocks. Fixes #735
2 parents 21ec2b1 + cfb044c commit d060e6e

File tree

8 files changed

+48
-28
lines changed

8 files changed

+48
-28
lines changed

CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,13 @@ server side, so if the feature is enabled, router version 1.6.2+ will be require
99

1010
## Issues Fixed and Dependency Updates
1111

12+
# Release notes 1.1.1
13+
14+
## Issues Fixed and Dependency Updates
15+
1216
* github.com/openziti/sdk-golang: [v1.1.0 -> v1.1.1](https://github.com/openziti/sdk-golang/compare/v1.1.0...v1.1.1)
17+
* [Issue #735](https://github.com/openziti/sdk-golang/issues/735) - Ensure Authenticate can't be called in parallel
18+
1319
* github.com/openziti/channel/v4: [v4.0.6 -> v4.1.3](https://github.com/openziti/channel/compare/v4.0.6...v4.1.3)
1420
* [Issue #187](https://github.com/openziti/channel/issues/187) - Allow fallback to regular channel when 'is grouped' isn't set when using multi-listener
1521
* [Issue #185](https://github.com/openziti/channel/issues/185) - Add group secret for multi-underlay channels
@@ -26,6 +32,7 @@ server side, so if the feature is enabled, router version 1.6.2+ will be require
2632
* golang.org/x/term: v0.30.0 -> v0.32.0
2733
* golang.org/x/text: v0.23.0 -> v0.25.0
2834

35+
2936
# Release notes 1.1.0
3037

3138
## What's New

example/go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ require (
8080
github.com/muhlemmer/gu v0.3.1 // indirect
8181
github.com/oklog/ulid v1.3.1 // indirect
8282
github.com/opentracing/opentracing-go v1.2.0 // indirect
83-
github.com/openziti/channel/v4 v4.1.3 // indirect
83+
github.com/openziti/channel/v4 v4.2.0 // indirect
8484
github.com/openziti/edge-api v0.26.45 // indirect
8585
github.com/openziti/identity v1.0.101 // indirect
8686
github.com/openziti/metrics v1.4.1 // indirect

example/go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -358,8 +358,8 @@ github.com/onsi/gomega v1.13.0 h1:7lLHu94wT9Ij0o6EWWclhu0aOh32VxhkwEJvzuWPeak=
358358
github.com/onsi/gomega v1.13.0/go.mod h1:lRk9szgn8TxENtWd0Tp4c3wjlRfMTMH27I+3Je41yGY=
359359
github.com/opentracing/opentracing-go v1.2.0 h1:uEJPy/1a5RIPAJ0Ov+OIO8OxWu77jEv+1B0VhjKrZUs=
360360
github.com/opentracing/opentracing-go v1.2.0/go.mod h1:GxEUsuufX4nBwe+T+Wl9TAgYrxe9dPLANfrWvHYVTgc=
361-
github.com/openziti/channel/v4 v4.1.3 h1:+weirY7+Yxw45v4yX9lwO3W7/Wqv/fTFuErk1J2mmu4=
362-
github.com/openziti/channel/v4 v4.1.3/go.mod h1:t42XcuNICtJSizetoZbmml3l5AE7d/LDCf29c+Ene4Q=
361+
github.com/openziti/channel/v4 v4.2.0 h1:oDWDECsMlGqYOWTR7cpvWiEtDgK2zH5XYyU94mMobUQ=
362+
github.com/openziti/channel/v4 v4.2.0/go.mod h1:t42XcuNICtJSizetoZbmml3l5AE7d/LDCf29c+Ene4Q=
363363
github.com/openziti/edge-api v0.26.45 h1:M5rR28yEIIpbVo7F7c5tF+z2qNKqvN55pK6bZqQHn+8=
364364
github.com/openziti/edge-api v0.26.45/go.mod h1:sYHVpm26Jr1u7VooNJzTb2b2nGSlmCHMnbGC8XfWSng=
365365
github.com/openziti/foundation/v2 v2.0.63 h1:7D8JhHT3i4H5owF+XnTdyjCKn809xEcFD8/RG1h9QYA=

example/influxdb-client-go/go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ require (
9494
github.com/muhlemmer/gu v0.3.1 // indirect
9595
github.com/oklog/ulid v1.3.1 // indirect
9696
github.com/opentracing/opentracing-go v1.2.0 // indirect
97-
github.com/openziti/channel/v4 v4.1.3 // indirect
97+
github.com/openziti/channel/v4 v4.2.0 // indirect
9898
github.com/openziti/edge-api v0.26.45 // indirect
9999
github.com/openziti/foundation/v2 v2.0.63 // indirect
100100
github.com/openziti/identity v1.0.101 // indirect

example/influxdb-client-go/go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -413,8 +413,8 @@ github.com/onsi/gomega v1.13.0 h1:7lLHu94wT9Ij0o6EWWclhu0aOh32VxhkwEJvzuWPeak=
413413
github.com/onsi/gomega v1.13.0/go.mod h1:lRk9szgn8TxENtWd0Tp4c3wjlRfMTMH27I+3Je41yGY=
414414
github.com/opentracing/opentracing-go v1.2.0 h1:uEJPy/1a5RIPAJ0Ov+OIO8OxWu77jEv+1B0VhjKrZUs=
415415
github.com/opentracing/opentracing-go v1.2.0/go.mod h1:GxEUsuufX4nBwe+T+Wl9TAgYrxe9dPLANfrWvHYVTgc=
416-
github.com/openziti/channel/v4 v4.1.3 h1:+weirY7+Yxw45v4yX9lwO3W7/Wqv/fTFuErk1J2mmu4=
417-
github.com/openziti/channel/v4 v4.1.3/go.mod h1:t42XcuNICtJSizetoZbmml3l5AE7d/LDCf29c+Ene4Q=
416+
github.com/openziti/channel/v4 v4.2.0 h1:oDWDECsMlGqYOWTR7cpvWiEtDgK2zH5XYyU94mMobUQ=
417+
github.com/openziti/channel/v4 v4.2.0/go.mod h1:t42XcuNICtJSizetoZbmml3l5AE7d/LDCf29c+Ene4Q=
418418
github.com/openziti/edge-api v0.26.45 h1:M5rR28yEIIpbVo7F7c5tF+z2qNKqvN55pK6bZqQHn+8=
419419
github.com/openziti/edge-api v0.26.45/go.mod h1:sYHVpm26Jr1u7VooNJzTb2b2nGSlmCHMnbGC8XfWSng=
420420
github.com/openziti/foundation/v2 v2.0.63 h1:7D8JhHT3i4H5owF+XnTdyjCKn809xEcFD8/RG1h9QYA=

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ require (
1818
github.com/michaelquigley/pfxlog v0.6.10
1919
github.com/mitchellh/go-ps v1.0.0
2020
github.com/mitchellh/mapstructure v1.5.0
21-
github.com/openziti/channel/v4 v4.1.3
21+
github.com/openziti/channel/v4 v4.2.0
2222
github.com/openziti/edge-api v0.26.45
2323
github.com/openziti/foundation/v2 v2.0.63
2424
github.com/openziti/identity v1.0.101

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -299,8 +299,8 @@ github.com/onsi/gomega v1.13.0 h1:7lLHu94wT9Ij0o6EWWclhu0aOh32VxhkwEJvzuWPeak=
299299
github.com/onsi/gomega v1.13.0/go.mod h1:lRk9szgn8TxENtWd0Tp4c3wjlRfMTMH27I+3Je41yGY=
300300
github.com/opentracing/opentracing-go v1.2.0 h1:uEJPy/1a5RIPAJ0Ov+OIO8OxWu77jEv+1B0VhjKrZUs=
301301
github.com/opentracing/opentracing-go v1.2.0/go.mod h1:GxEUsuufX4nBwe+T+Wl9TAgYrxe9dPLANfrWvHYVTgc=
302-
github.com/openziti/channel/v4 v4.1.3 h1:+weirY7+Yxw45v4yX9lwO3W7/Wqv/fTFuErk1J2mmu4=
303-
github.com/openziti/channel/v4 v4.1.3/go.mod h1:t42XcuNICtJSizetoZbmml3l5AE7d/LDCf29c+Ene4Q=
302+
github.com/openziti/channel/v4 v4.2.0 h1:oDWDECsMlGqYOWTR7cpvWiEtDgK2zH5XYyU94mMobUQ=
303+
github.com/openziti/channel/v4 v4.2.0/go.mod h1:t42XcuNICtJSizetoZbmml3l5AE7d/LDCf29c+Ene4Q=
304304
github.com/openziti/edge-api v0.26.45 h1:M5rR28yEIIpbVo7F7c5tF+z2qNKqvN55pK6bZqQHn+8=
305305
github.com/openziti/edge-api v0.26.45/go.mod h1:sYHVpm26Jr1u7VooNJzTb2b2nGSlmCHMnbGC8XfWSng=
306306
github.com/openziti/foundation/v2 v2.0.63 h1:7D8JhHT3i4H5owF+XnTdyjCKn809xEcFD8/RG1h9QYA=

ziti/ziti.go

Lines changed: 32 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,7 @@ type ContextImpl struct {
193193
authQueryHandlers map[string]func(query *rest_model.AuthQueryDetail, response MfaCodeResponse) error
194194

195195
events.EventEmmiter
196+
authAttemptLock sync.Mutex
196197
lastSuccessfulApiSessionRefresh time.Time
197198
routerProxy func(addr string) *transport.ProxyConfiguration
198199

@@ -664,41 +665,50 @@ func (context *ContextImpl) refreshSessions() {
664665
}
665666

666667
func (context *ContextImpl) RefreshServices() error {
667-
return context.refreshServices(true)
668+
return context.refreshServices(true, false)
668669
}
669670

670-
func (context *ContextImpl) refreshServices(forceCheck bool) error {
671-
if err := context.ensureApiSession(); err != nil {
672-
return fmt.Errorf("failed to refresh services: %v", err)
671+
func (context *ContextImpl) refreshServices(forceRefresh, refreshAfterAuth bool) error {
672+
if !refreshAfterAuth { // if in authenticate mutex, can't re-auth
673+
if err := context.ensureApiSession(); err != nil {
674+
return fmt.Errorf("failed to refresh services: %v", err)
675+
}
673676
}
674677

675678
var checkService bool
676679
var lastServiceUpdate *strfmt.DateTime
677680
var err error
678681

679682
log := pfxlog.Logger()
680-
log.Debug("checking if service updates available")
681-
if checkService, lastServiceUpdate, err = context.CtrlClt.IsServiceListUpdateAvailable(); err != nil {
682-
log.WithError(err).Error("failed to check if service list update is available")
683-
target := &current_api_session.ListServiceUpdatesUnauthorized{}
684-
if errors.As(err, &target) {
685-
checkService = true
686-
} else {
687-
if err = context.Authenticate(); err != nil {
688-
log.WithError(err).Error("unable to re-authenticate during session refresh")
683+
684+
if !forceRefresh {
685+
log.Debug("checking if service updates available")
686+
if checkService, lastServiceUpdate, err = context.CtrlClt.IsServiceListUpdateAvailable(); err != nil {
687+
log.WithError(err).Error("failed to check if service list update is available")
688+
target := &current_api_session.ListServiceUpdatesUnauthorized{}
689+
if errors.As(err, &target) {
690+
checkService = true
689691
} else {
690-
if checkService, lastServiceUpdate, err = context.CtrlClt.IsServiceListUpdateAvailable(); err != nil {
691-
checkService = true
692+
if err = context.Authenticate(); err != nil {
693+
log.WithError(err).Error("unable to re-authenticate during session refresh")
694+
} else {
695+
if checkService, lastServiceUpdate, err = context.CtrlClt.IsServiceListUpdateAvailable(); err != nil {
696+
checkService = true
697+
}
692698
}
693699
}
694700
}
695701
}
696702

697-
if checkService || forceCheck {
703+
if checkService || forceRefresh {
698704
log.Debug("refreshing services")
699705

700706
services, err := context.CtrlClt.GetServices()
701707
if err != nil {
708+
if refreshAfterAuth { // in authenticate mutex, can't re-auth
709+
return err
710+
}
711+
702712
target := &service.ListServicesUnauthorized{}
703713
if errors.As(err, &target) {
704714
log.Info("attempting to re-authenticate")
@@ -831,7 +841,7 @@ func (context *ContextImpl) runRefreshes() {
831841

832842
case <-svcRefreshTick.C:
833843
log.Debug("refreshing services")
834-
if err := context.refreshServices(false); err != nil {
844+
if err := context.refreshServices(false, false); err != nil {
835845
log.WithError(err).Error("failed to load service updates")
836846
}
837847

@@ -939,6 +949,9 @@ func (context *ContextImpl) Reauthenticate() error {
939949
}
940950

941951
func (context *ContextImpl) Authenticate() error {
952+
context.authAttemptLock.Lock()
953+
defer context.authAttemptLock.Unlock()
954+
942955
if context.CtrlClt.GetCurrentApiSession() != nil {
943956
if time.Since(context.lastSuccessfulApiSessionRefresh) < 5*time.Second {
944957
return nil
@@ -1013,7 +1026,7 @@ func (context *ContextImpl) onFullAuth(apiSession apis.ApiSession) error {
10131026
context.Emit(EventAuthenticationStateFull, apiSession)
10141027

10151028
// get services
1016-
if err := context.RefreshServices(); err != nil {
1029+
if err := context.refreshServices(true, true); err != nil {
10171030
doOnceErr = err
10181031
}
10191032

@@ -1668,7 +1681,7 @@ func (context *ContextImpl) createSession(service *rest_model.ServiceDetail, ses
16681681

16691682
var createSessionNotFound = &rest_session.CreateSessionNotFound{}
16701683
if errors.As(err, &createSessionNotFound) {
1671-
if refreshErr := context.refreshServices(false); refreshErr != nil {
1684+
if refreshErr := context.refreshServices(false, false); refreshErr != nil {
16721685
logger.WithError(refreshErr).Info("failed to refresh services after create session returned 404 (likely for service)")
16731686
}
16741687
}

0 commit comments

Comments
 (0)