fix: use singleflight to prevent concurrent map race in ResolveEndpoints

The code in ResolveEndpoints read m.cache without holding m.mu after
addCachedEndpoints released the lock, causing a fatal concurrent map
read/write when multiple goroutines resolved the same authority.

Use golang.org/x/sync/singleflight to deduplicate concurrent discovery
calls by authority URI. This ensures only one goroutine performs
discovery per authority at a time, eliminating the race and redundant
network calls. addCachedEndpoints now returns the aliases so callers
never access the map without the lock.

Fixes #620

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: bgavrilMS

apps/internal/oauth/resolvers.go

@@ -16,6 +16,7 @@ import (
 
 	"github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/oauth/ops"
 	"github.com/AzureAD/microsoft-authentication-library-for-go/apps/internal/oauth/ops/authority"
+	"golang.org/x/sync/singleflight"
 )
 
 type cacheEntry struct {
@@ -35,6 +36,8 @@ type authorityEndpoint struct {
 
 	mu    sync.Mutex
 	cache map[string]cacheEntry
+
+	resolveGroup singleflight.Group
 }
 
 // newAuthorityEndpoint is the constructor for AuthorityEndpoint.
@@ -50,35 +53,49 @@ func (m *authorityEndpoint) ResolveEndpoints(ctx context.Context, authorityInfo
 		return endpoints, nil
 	}
 
-	endpoint, err := m.openIDConfigurationEndpoint(ctx, authorityInfo)
-	if err != nil {
-		return authority.Endpoints{}, err
-	}
+	key := authorityInfo.CanonicalAuthorityURI
+	v, err, _ := m.resolveGroup.Do(key, func() (interface{}, error) {
+		// Double-check inside the singleflight group: another goroutine may
+		// have populated the cache while we were waiting.
+		if endpoints, found := m.cachedEndpoints(authorityInfo, userPrincipalName); found {
+			return endpoints, nil
+		}
 
-	resp, err := m.rest.Authority().GetTenantDiscoveryResponse(ctx, endpoint)
-	if err != nil {
-		return authority.Endpoints{}, err
-	}
-	if err := resp.Validate(); err != nil {
-		return authority.Endpoints{}, fmt.Errorf("ResolveEndpoints(): %w", err)
-	}
+		endpoint, err := m.openIDConfigurationEndpoint(ctx, authorityInfo)
+		if err != nil {
+			return authority.Endpoints{}, err
+		}
+
+		resp, err := m.rest.Authority().GetTenantDiscoveryResponse(ctx, endpoint)
+		if err != nil {
+			return authority.Endpoints{}, err
+		}
+		if err := resp.Validate(); err != nil {
+			return authority.Endpoints{}, fmt.Errorf("ResolveEndpoints(): %w", err)
+		}
 
-	tenant := authorityInfo.Tenant
+		tenant := authorityInfo.Tenant
 
-	endpoints := authority.NewEndpoints(
-		strings.Replace(resp.AuthorizationEndpoint, "{tenant}", tenant, -1),
-		strings.Replace(resp.TokenEndpoint, "{tenant}", tenant, -1),
-		strings.Replace(resp.Issuer, "{tenant}", tenant, -1),
-		authorityInfo.Host)
+		endpoints := authority.NewEndpoints(
+			strings.Replace(resp.AuthorizationEndpoint, "{tenant}", tenant, -1),
+			strings.Replace(resp.TokenEndpoint, "{tenant}", tenant, -1),
+			strings.Replace(resp.Issuer, "{tenant}", tenant, -1),
+			authorityInfo.Host)
 
-	m.addCachedEndpoints(authorityInfo, userPrincipalName, endpoints)
+		aliases := m.addCachedEndpoints(authorityInfo, userPrincipalName, endpoints)
 
-	if err := resp.ValidateIssuerMatchesAuthority(authorityInfo.CanonicalAuthorityURI,
-		m.cache[authorityInfo.CanonicalAuthorityURI].Aliases); err != nil {
-		return authority.Endpoints{}, fmt.Errorf("ResolveEndpoints(): %w", err)
+		if err := resp.ValidateIssuerMatchesAuthority(authorityInfo.CanonicalAuthorityURI,
+			aliases); err != nil {
+			return authority.Endpoints{}, fmt.Errorf("ResolveEndpoints(): %w", err)
+		}
+
+		return endpoints, nil
+	})
+	if err != nil {
+		return authority.Endpoints{}, err
 	}
 
-	return endpoints, nil
+	return v.(authority.Endpoints), nil
 }
 
 // cachedEndpoints returns the cached endpoints if they exist. If not, we return false.
@@ -100,7 +117,7 @@ func (m *authorityEndpoint) cachedEndpoints(authorityInfo authority.Info, userPr
 	return authority.Endpoints{}, false
 }
 
-func (m *authorityEndpoint) addCachedEndpoints(authorityInfo authority.Info, userPrincipalName string, endpoints authority.Endpoints) {
+func (m *authorityEndpoint) addCachedEndpoints(authorityInfo authority.Info, userPrincipalName string, endpoints authority.Endpoints) map[string]bool {
 	m.mu.Lock()
 	defer m.mu.Unlock()
 
@@ -128,6 +145,7 @@ func (m *authorityEndpoint) addCachedEndpoints(authorityInfo authority.Info, use
 	}
 
 	m.cache[authorityInfo.CanonicalAuthorityURI] = updatedCacheEntry
+	return updatedCacheEntry.Aliases
 }
 
 func (m *authorityEndpoint) openIDConfigurationEndpoint(ctx context.Context, authorityInfo authority.Info) (string, error) {

go.mod

@@ -8,6 +8,7 @@ require (
 	github.com/kylelemons/godebug v1.1.0
 	github.com/montanaflynn/stats v0.7.0
 	github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8
+	golang.org/x/sync v0.10.0
 )
 
 require golang.org/x/sys v0.5.0 // indirect

go.sum

@@ -8,6 +8,8 @@ github.com/montanaflynn/stats v0.7.0 h1:r3y12KyNxj/Sb/iOE46ws+3mS1+MZca1wlHQFPsY
 github.com/montanaflynn/stats v0.7.0/go.mod h1:etXPPgVO6n31NxCd9KQUMvCM+ve0ruNzt6R8Bnaayow=
 github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8 h1:KoWmjvw+nsYOo29YJK9vDA65RGE3NrOnUtO7a+RF9HU=
 github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8/go.mod h1:HKlIX3XHQyzLZPlr7++PzdhaXEj94dEiJgZDTsxEqUI=
+golang.org/x/sync v0.10.0 h1:3NQrjDixjgGwUOCaF8w2+VYHv0Ve/vGYSbdkTa98gmQ=
+golang.org/x/sync v0.10.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
 golang.org/x/sys v0.0.0-20210616045830-e2b7044e8c71/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.5.0 h1:MUK/U/4lj1t1oPg0HfuXDN/Z1wv31ZJ/YcPiGccS4DU=
 golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=