auth: CachingCredentialsSource is concurrency-safe

Previously CachingCredentialsSource was not safe for concurrent use, but
was not documented as such so callers tried to use it concurrently anyway.

It's realtively straightfoward to use a mutex to guard the internal cache
map, so we'll do that here to avoid pushing the synchronization
responsibility downstream into callers.

This mutex is here only to prevent corrupting our map and it intentionally
does not prevent concurrent calls to the underlying credentials source.
This means that two concurrent callers might sometimes race to populate
the same cache entry, in which case it's unspecified which one will "win"
and have its result retained for future calls, but one of them definitely
will.

auth/cache.go

@@ -3,7 +3,9 @@
 package auth
 
 import (
-	"github.com/hashicorp/terraform-svchost"
+	"sync"
+
+	svchost "github.com/hashicorp/terraform-svchost"
 )
 
 // CachingCredentialsSource creates a new credentials source that wraps another
@@ -23,6 +25,7 @@ func CachingCredentialsSource(source CredentialsSource) CredentialsSource {
 type cachingCredentialsSource struct {
 	source CredentialsSource
 	cache  map[svchost.Hostname]HostCredentials
+	mu     sync.Mutex
 }
 
 // ForHost passes the given hostname on to the wrapped credentials source and
@@ -33,31 +36,40 @@ type cachingCredentialsSource struct {
 // No cache entry is created if the wrapped source returns an error, to allow
 // the caller to retry the failing operation.
 func (s *cachingCredentialsSource) ForHost(host svchost.Hostname) (HostCredentials, error) {
+	s.mu.Lock()
 	if cache, cached := s.cache[host]; cached {
+		s.mu.Unlock()
 		return cache, nil
 	}
+	s.mu.Unlock()
 
 	result, err := s.source.ForHost(host)
 	if err != nil {
 		return result, err
 	}
 
+	s.mu.Lock()
 	s.cache[host] = result
+	s.mu.Unlock()
 	return result, nil
 }
 
 func (s *cachingCredentialsSource) StoreForHost(host svchost.Hostname, credentials HostCredentialsWritable) error {
 	// We'll delete the cache entry even if the store fails, since that just
 	// means that the next read will go to the real store and get a chance to
 	// see which object (old or new) is actually present.
+	s.mu.Lock()
 	delete(s.cache, host)
+	s.mu.Unlock()
 	return s.source.StoreForHost(host, credentials)
 }
 
 func (s *cachingCredentialsSource) ForgetForHost(host svchost.Hostname) error {
 	// We'll delete the cache entry even if the store fails, since that just
 	// means that the next read will go to the real store and get a chance to
 	// see if the object is still present.
+	s.mu.Lock()
 	delete(s.cache, host)
+	s.mu.Unlock()
 	return s.source.ForgetForHost(host)
 }