[Fix] IAM: Make agency AK/SK reauth thread-safe

openstack/client.go

@@ -281,20 +281,26 @@ func v3AKSKAuth(client *golangsdk.ProviderClient, endpoint string, options golan
 	// update AKSKAuthOptions of ProviderClient
 	// ProviderClient(client) is a reference to the ServiceClient(v3Client)
 	defer func() {
-		client.AKSKAuthOptions.ProjectId = options.ProjectId
-		client.AKSKAuthOptions.DomainID = options.DomainID
+		client.UpdateAKSKOptions(func(o *golangsdk.AKSKAuthOptions) {
+			o.ProjectId = options.ProjectId
+			o.DomainID = options.DomainID
+		})
 	}()
 
-	client.AKSKAuthOptions = options
-	client.AKSKAuthOptions.DomainID = ""
+	client.UpdateAKSKOptions(func(o *golangsdk.AKSKAuthOptions) {
+		*o = options
+		o.DomainID = ""
+	})
 
 	if options.ProjectId == "" && options.ProjectName != "" {
 		id, err := getProjectID(v3Client, options.ProjectName)
 		if err != nil {
 			return err
 		}
 		options.ProjectId = id
-		client.AKSKAuthOptions.ProjectId = options.ProjectId
+		client.UpdateAKSKOptions(func(o *golangsdk.AKSKAuthOptions) {
+			o.ProjectId = options.ProjectId
+		})
 	}
 
 	if options.DomainID == "" && options.Domain != "" {
@@ -368,27 +374,33 @@ func authWithAgencyByAKSK(client *golangsdk.ProviderClient, endpoint string, opt
 		return fmt.Errorf("error obtaining temporary AK/SK for agency: %w", err)
 	}
 
-	client.AKSKAuthOptions.AccessKey = credential.AccessKey
-	client.AKSKAuthOptions.SecretKey = credential.SecretKey
-	client.AKSKAuthOptions.SecurityToken = credential.SecurityToken
-	client.AKSKAuthOptions.ProjectId = ""
-	client.AKSKAuthOptions.DomainID = ""
+	client.UpdateAKSKOptions(func(o *golangsdk.AKSKAuthOptions) {
+		o.AccessKey = credential.AccessKey
+		o.SecretKey = credential.SecretKey
+		o.SecurityToken = credential.SecurityToken
+		o.ProjectId = ""
+		o.DomainID = ""
+	})
 
 	if opts.DelegatedProject != "" {
 		projectID, err := getProjectID(v3Client, opts.DelegatedProject)
 		if err != nil {
 			return fmt.Errorf("error resolving delegated project %q: %w", opts.DelegatedProject, err)
 		}
 		client.ProjectID = projectID
-		client.AKSKAuthOptions.ProjectId = projectID
+		client.UpdateAKSKOptions(func(o *golangsdk.AKSKAuthOptions) {
+			o.ProjectId = projectID
+		})
 	} else {
 		domainID, err := getDomainID(opts.AgencyDomainName, v3Client)
 		if err != nil {
 			return fmt.Errorf("error resolving agency domain %q: %w", opts.AgencyDomainName, err)
 		}
 		client.ProjectID = ""
 		client.DomainID = domainID
-		client.AKSKAuthOptions.DomainID = domainID
+		client.UpdateAKSKOptions(func(o *golangsdk.AKSKAuthOptions) {
+			o.DomainID = domainID
+		})
 	}
 
 	client.ReauthFunc = func() error {

openstack/testing/client_test.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"net/http"
 	"strings"
+	"sync"
 	"testing"
 
 	"github.com/opentelekomcloud/gophertelekomcloud"
@@ -490,3 +491,133 @@ func TestAuthenticatedClientV3WithAgencyAKSKWithoutDelegatedProjectKeepsDomainSc
 	th.CheckEquals(t, "temporary-ak", client.AKSKAuthOptions.AccessKey)
 	th.CheckEquals(t, "temporary-security-token", client.AKSKAuthOptions.SecurityToken)
 }
+
+// TestAuthenticatedClientV3AgencyAKSKConcurrentReauthIsRaceFree drives many concurrent signed
+// requests while the agency ReauthFunc keeps refreshing the temporary AK/SK. Run with -race
+func TestAuthenticatedClientV3AgencyAKSKConcurrentReauthIsRaceFree(t *testing.T) {
+	th.SetupHTTP()
+	defer th.TeardownHTTP()
+
+	th.Mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
+		_, _ = fmt.Fprintf(w, `
+			{
+				"versions": {
+					"values": [
+						{
+							"status": "stable",
+							"id": "v3.0",
+							"links": [
+								{ "href": "%s", "rel": "self" }
+							]
+						}
+					]
+				}
+			}
+		`, th.Endpoint()+"v3/")
+	})
+
+	th.Mux.HandleFunc("/v3/auth/catalog", func(w http.ResponseWriter, r *http.Request) {
+		_, _ = fmt.Fprintf(w, `
+			{
+				"catalog": [
+					{
+						"type": "identity",
+						"name": "iam",
+						"endpoints": [
+							{
+								"interface": "public",
+								"region": "eu-de",
+								"url": "%s"
+							}
+						]
+					}
+				],
+				"links": { "next": null, "previous": null }
+			}
+		`, th.Endpoint()+"v3/")
+	})
+
+	th.Mux.HandleFunc("/v3.0/OS-CREDENTIAL/securitytokens", func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusCreated)
+		_, _ = fmt.Fprint(w, `
+			{
+				"credential": {
+					"access": "temporary-ak",
+					"secret": "temporary-sk",
+					"securitytoken": "temporary-security-token",
+					"expires_at": "2030-01-01T00:00:00.000000Z"
+				}
+			}
+		`)
+	})
+
+	th.Mux.HandleFunc("/v3/projects", func(w http.ResponseWriter, r *http.Request) {
+		if r.Header.Get("Authorization") == "" {
+			t.Errorf("expected request to be signed")
+		}
+		_, _ = fmt.Fprint(w, `
+			{
+				"projects": [
+					{
+						"id": "target-project-id",
+						"name": "target-project"
+					}
+				],
+				"links": { "next": null, "previous": null }
+			}
+		`)
+	})
+
+	options := golangsdk.AKSKAuthOptions{
+		IdentityEndpoint: th.Endpoint(),
+		DomainID:         "source-domain-id",
+		AccessKey:        "source-ak",
+		SecretKey:        "source-sk",
+		AgencyName:       "target-agency",
+		AgencyDomainName: "target-domain",
+		DelegatedProject: "target-project",
+	}
+
+	client, err := openstack.AuthenticatedClient(options)
+	th.AssertNoErr(t, err)
+
+	const (
+		readers    = 16
+		iterations = 50
+	)
+
+	var wg sync.WaitGroup
+
+	// Single writer goroutine, mirroring the serialized reauth in reauthenticateAndRetry: it keeps
+	// publishing fresh temporary credentials while the readers below sign concurrently.
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+		for i := 0; i < iterations; i++ {
+			if err := client.ReauthFunc(); err != nil {
+				t.Errorf("reauth failed: %s", err)
+				return
+			}
+		}
+	}()
+
+	endpoint := th.Endpoint() + "v3/projects"
+	for r := 0; r < readers; r++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			for i := 0; i < iterations; i++ {
+				resp, err := client.Request("GET", endpoint, &golangsdk.RequestOpts{
+					OkCodes: []int{200},
+				})
+				if err != nil {
+					t.Errorf("signed request failed: %s", err)
+					return
+				}
+				_ = resp.Body.Close()
+			}
+		}()
+	}
+
+	wg.Wait()
+}

provider_client.go

@@ -96,6 +96,10 @@ type ProviderClient struct {
 	mut *sync.RWMutex
 
 	reauthmut *reauthlock
+
+	// akskMut guards concurrent access to AKSKAuthOptions. It allows request signing to read
+	// a consistent snapshot of the AK/SK credentials while a ReauthFunc refreshes them.
+	akskMut *sync.RWMutex
 }
 
 type reauthlock struct {
@@ -126,6 +130,30 @@ func (client *ProviderClient) AuthenticatedHeaders() (m map[string]string) {
 func (client *ProviderClient) UseTokenLock() {
 	client.mut = new(sync.RWMutex)
 	client.reauthmut = new(reauthlock)
+	client.akskMut = new(sync.RWMutex)
+}
+
+// AKSKOptions safely reads a consistent snapshot of the AK/SK auth options. Applications and custom
+// ReauthFuncs should use this together with UpdateAKSKOptions instead of accessing the
+// AKSKAuthOptions field directly when the client is used concurrently.
+func (client *ProviderClient) AKSKOptions() AKSKAuthOptions {
+	if client.akskMut != nil {
+		client.akskMut.RLock()
+		defer client.akskMut.RUnlock()
+	}
+	return client.AKSKAuthOptions
+}
+
+// UpdateAKSKOptions safely mutates the AK/SK auth options under the write lock. It is intended for
+// use from a custom ReauthFunc to publish refreshed temporary credentials without racing concurrent
+// request signing. The callback must not perform any I/O, as that would hold the lock across the
+// request path and deadlock concurrent readers.
+func (client *ProviderClient) UpdateAKSKOptions(update func(opts *AKSKAuthOptions)) {
+	if client.akskMut != nil {
+		client.akskMut.Lock()
+		defer client.akskMut.Unlock()
+	}
+	update(&client.AKSKAuthOptions)
 }
 
 // Token safely reads the value of the auth token from the ProviderClient. Applications should
@@ -288,19 +316,20 @@ func (client *ProviderClient) Request(method, url string, options *RequestOpts)
 
 	prereqtok := req.Header.Get("X-Auth-Token")
 
-	if client.AKSKAuthOptions.AccessKey != "" {
+	akskOpts := client.AKSKOptions()
+	if akskOpts.AccessKey != "" {
 		Sign(req, SignOptions{
-			AccessKey: client.AKSKAuthOptions.AccessKey,
-			SecretKey: client.AKSKAuthOptions.SecretKey,
+			AccessKey: akskOpts.AccessKey,
+			SecretKey: akskOpts.SecretKey,
 		})
-		if client.AKSKAuthOptions.ProjectId != "" && client.AKSKAuthOptions.DomainID == "" {
-			req.Header.Set("X-Project-Id", client.AKSKAuthOptions.ProjectId)
+		if akskOpts.ProjectId != "" && akskOpts.DomainID == "" {
+			req.Header.Set("X-Project-Id", akskOpts.ProjectId)
 		}
-		if client.AKSKAuthOptions.DomainID != "" {
-			req.Header.Set("X-Domain-Id", client.AKSKAuthOptions.DomainID)
+		if akskOpts.DomainID != "" {
+			req.Header.Set("X-Domain-Id", akskOpts.DomainID)
 		}
-		if client.AKSKAuthOptions.SecurityToken != "" {
-			req.Header.Set("X-Security-Token", client.AKSKAuthOptions.SecurityToken)
+		if akskOpts.SecurityToken != "" {
+			req.Header.Set("X-Security-Token", akskOpts.SecurityToken)
 		}
 	}