Skip to content

[Fix] IAM: Fix cross-tenant agency AK/SK auth #955

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
anton-sidelnikov merged 3 commits into from
Jul 6, 2026
Merged

[Fix] IAM: Fix cross-tenant agency AK/SK auth #955

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3dbac75
[Fix] IAM: Use temporary AK/SK for cross-tenant agency auth
TobiWo Jun 29, 2026
9bf7f00
[Fix] IAM: Skip forbidden catalog re-fetch on agency AK/SK reauth
TobiWo Jun 30, 2026
2c5a12f
[Fix] IAM: Rework agency second pass per review
TobiWo Jul 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 27 additions & 35 deletions openstack/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

golangsdk "github.com/opentelekomcloud/gophertelekomcloud"
"github.com/opentelekomcloud/gophertelekomcloud/openstack/identity/v3/catalog"
"github.com/opentelekomcloud/gophertelekomcloud/openstack/identity/v3/credentials"
"github.com/opentelekomcloud/gophertelekomcloud/openstack/identity/v3/domains"
"github.com/opentelekomcloud/gophertelekomcloud/openstack/identity/v3/projects"
tokens3 "github.com/opentelekomcloud/gophertelekomcloud/openstack/identity/v3/tokens"
Expand Down Expand Up @@ -358,50 +359,41 @@ func authWithAgencyByAKSK(client *golangsdk.ProviderClient, endpoint string, opt
return fmt.Errorf("must config domain name")
}

opts2 := golangsdk.AgencyAuthOptions{
AgencyName: opts.AgencyName,
AgencyDomainName: opts.AgencyDomainName,
DelegatedProject: opts.DelegatedProject,
}
result := tokens3.Create(v3Client, &opts2)
token, err := result.ExtractToken()
if err != nil {
return err
}

project, err := result.ExtractProject()
if err != nil {
return fmt.Errorf("error extracting project info: %s", err)
}

user, err := result.ExtractUser()
credential, err := credentials.CreateTemporary(v3Client, credentials.CreateTemporaryOpts{
Methods: []string{"assume_role"},
DomainName: opts.AgencyDomainName,
AgencyName: opts.AgencyName,
}).Extract()
if err != nil {
return fmt.Errorf("error extracting user info: %s", err)
return fmt.Errorf("error obtaining temporary AK/SK for agency: %w", err)
}

serviceCatalog, err := result.ExtractServiceCatalog()
if err != nil {
return err
}
client.AKSKAuthOptions.AccessKey = credential.AccessKey
client.AKSKAuthOptions.SecretKey = credential.SecretKey
client.AKSKAuthOptions.SecurityToken = credential.SecurityToken
client.AKSKAuthOptions.ProjectId = ""
client.AKSKAuthOptions.DomainID = ""

client.TokenID = token.ID
if project != nil {
client.ProjectID = project.ID
}
if user != nil {
client.UserID = user.ID
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
} 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.ReauthFunc = func() error {

@artem-lifshits artem-lifshits Jul 3, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This introduces a data race: ReauthFunc rewrites client.AKSKAuthOptions while every request reads it unlocked when signing (provider_client.go:291-305).

The old code never hit this path because it cleared AccessKey. go test -race shows it right away.

@TobiWo TobiWo Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The race is real. Signing reads client.AKSKAuthOptions.* unlocked (provider_client.go:291-303), and ReauthFunc writes those same fields. reauthenticateAndRetry holds client.mut around ReauthFunc, but a first-time request signs in doRequest without taking that lock, so a concurrent sign and reauth can hit it together. The old token path avoided this by clearing AccessKey and skipping the signing block; we can't, because retries need AccessKey to re-sign.

One caveat for the thread: the suggested replacement doesn't fully close it. It swaps a whole-struct assignment for field-by-field writes, which shrinks the window, but those writes are still unlocked and still race the unlocked reads.
There is no locked setter for AK/SK options; only SetToken/Token guard anything. go test -race passes here only because the tests never sign a request while a reauth is running.

Decision: take the snippet now for the smaller window, and track the real fix separately: a locked accessor around the AK/SK signing reads and reauth writes in provider_client.go. That touches the signing path for every service, so it's out of scope for a hotfix. Pre-existing gap, not something this PR introduced. I will create a follow-up issue if you agree.

@artem-lifshits artem-lifshits Jul 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed it's real but it's not pre-existing. This PR makes it reachable (old code cleared AccessKey and signing was skipped).
I'm fine to merge it and fix in the next issue.

client.TokenID = ""
return authWithAgencyByAKSK(client, endpoint, opts, eo)
}

client.EndpointLocator = func(opts golangsdk.EndpointOpts) (string, error) {
return V3EndpointURL(serviceCatalog, opts)
}

client.AKSKAuthOptions.AccessKey = ""
return nil
}

Expand Down
270 changes: 270 additions & 0 deletions openstack/testing/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package testing
import (
"fmt"
"net/http"
"strings"
"testing"

"github.com/opentelekomcloud/gophertelekomcloud"
Expand Down Expand Up @@ -220,3 +221,272 @@ func TestAuthenticatedClientV3Fails(t *testing.T) {
func TestAuthenticatedClientV2Fails(t *testing.T) {
testAuthenticatedClientFails(t, "http://bad-address.example.com/v2.0")
}

func TestAuthenticatedClientV3WithAgencyAKSKUsesTemporaryCredentials(t *testing.T) {
th.SetupHTTP()
defer th.TeardownHTTP()

var temporaryCredentialRequests int
var catalogRequests int

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) {
th.TestMethod(t, r, "GET")
if r.Header.Get("Authorization") == "" {
t.Errorf("expected request to be signed")
}
if strings.Contains(r.Header.Get("Authorization"), "Credential=temporary-ak") {
t.Errorf("catalog must not be fetched with assumed-role credentials, got %q", r.Header.Get("Authorization"))
}
catalogRequests++

_, _ = 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) {
temporaryCredentialRequests++
th.TestMethod(t, r, "POST")
th.TestHeader(t, r, "X-Domain-Id", "source-domain-id")
if r.Header.Get("Authorization") == "" {
t.Errorf("expected request to be signed")
}
th.TestJSONRequest(t, r, `
{
"auth": {
"identity": {
"methods": ["assume_role"],
"assume_role": {
"agency_name": "target-agency",
"domain_name": "target-domain"
}
}
}
}
`)

w.WriteHeader(http.StatusCreated)
_, _ = fmt.Fprintf(w, `
{
"credential": {
"access": "temporary-ak-%[1]d",
"secret": "temporary-sk-%[1]d",
"securitytoken": "temporary-security-token-%[1]d",
"expires_at": "2030-01-01T00:00:00.000000Z"
}
}
`, temporaryCredentialRequests)
})

th.Mux.HandleFunc("/v3/projects", func(w http.ResponseWriter, r *http.Request) {
th.TestMethod(t, r, "GET")
th.TestFormValues(t, r, map[string]string{"name": "target-project"})
if r.Header.Get("Authorization") == "" {
t.Errorf("expected request to be signed")
}
th.TestHeader(t, r, "X-Security-Token", fmt.Sprintf("temporary-security-token-%d", temporaryCredentialRequests))
if !strings.Contains(r.Header.Get("Authorization"), fmt.Sprintf("Credential=temporary-ak-%d/", temporaryCredentialRequests)) {
t.Errorf("expected temporary AK in Authorization header, got %q", r.Header.Get("Authorization"))
}

_, _ = 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)
th.CheckEquals(t, 1, temporaryCredentialRequests)
th.CheckEquals(t, 1, catalogRequests)
th.CheckEquals(t, "temporary-ak-1", client.AKSKAuthOptions.AccessKey)
th.CheckEquals(t, "temporary-sk-1", client.AKSKAuthOptions.SecretKey)
th.CheckEquals(t, "temporary-security-token-1", client.AKSKAuthOptions.SecurityToken)
th.CheckEquals(t, "target-project-id", client.ProjectID)

err = client.ReauthFunc()
th.AssertNoErr(t, err)
th.CheckEquals(t, 2, temporaryCredentialRequests)
th.CheckEquals(t, 2, catalogRequests)
th.CheckEquals(t, "temporary-ak-2", client.AKSKAuthOptions.AccessKey)
th.CheckEquals(t, "temporary-sk-2", client.AKSKAuthOptions.SecretKey)
th.CheckEquals(t, "temporary-security-token-2", client.AKSKAuthOptions.SecurityToken)
}

func TestAuthenticatedClientV3WithAgencyAKSKWithoutDelegatedProjectKeepsDomainScope(t *testing.T) {
th.SetupHTTP()
defer th.TeardownHTTP()

var catalogRequests int

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) {
if strings.Contains(r.Header.Get("Authorization"), "Credential=temporary-ak") {
t.Errorf("catalog must not be fetched with assumed-role credentials, got %q", r.Header.Get("Authorization"))
}
catalogRequests++
_, _ = 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) {
th.TestJSONRequest(t, r, `
{
"auth": {
"identity": {
"methods": ["assume_role"],
"assume_role": {
"agency_name": "target-agency",
"domain_name": "target-domain"
}
}
}
}
`)

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/auth/domains", func(w http.ResponseWriter, r *http.Request) {
th.TestMethod(t, r, "GET")
th.TestFormValues(t, r, map[string]string{"name": "target-domain"})
th.TestHeader(t, r, "X-Security-Token", "temporary-security-token")
if !strings.Contains(r.Header.Get("Authorization"), "Credential=temporary-ak/") {
t.Errorf("expected temporary AK in Authorization header, got %q", r.Header.Get("Authorization"))
}

_, _ = fmt.Fprint(w, `
{
"domains": [
{
"id": "target-domain-id",
"name": "target-domain"
}
],
"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",
}

client, err := openstack.AuthenticatedClient(options)
th.AssertNoErr(t, err)
th.CheckEquals(t, 1, catalogRequests)
th.CheckEquals(t, "target-domain-id", client.DomainID)
th.CheckEquals(t, "target-domain-id", client.AKSKAuthOptions.DomainID)
th.CheckEquals(t, "temporary-ak", client.AKSKAuthOptions.AccessKey)
th.CheckEquals(t, "temporary-security-token", client.AKSKAuthOptions.SecurityToken)
}
Loading