From c7198f1ddc3fec954334ae3f0e2336ca7b4c0c76 Mon Sep 17 00:00:00 2001 From: Travis Wu Date: Fri, 11 Sep 2026 11:42:35 +0800 Subject: [PATCH] openstack: say when a Keystone cannot redeem application credentials at all MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A lab validation against a CubeCOS cluster could not redeem an application credential that existed, was not revoked, and was scoped correctly. Keystone answered 401 with "Attempted to authenticate with an unsupported method" and the list it does support: password, token, oauth1, mapped. CubeCOS ships methods = password,token,oauth1,mapped in keystone.conf, so an application credential can be created there and never redeemed. The old message named the one thing that was not wrong — "check that it exists and has not been revoked" — and sent the reader to inspect a healthy credential. The two conditions have different owners: a revoked credential is the operator's to reissue, a missing auth method is the platform's to enable. Only the method names are read from the error body, never error.message and never the body itself, because a Keystone error can echo the request and the request holds the secret. An ordinary 401 keeps its original wording, pinned by its own test so the new branch cannot swallow it. Signed-off-by: Travis Wu --- internal/openstack/compute.go | 39 ++++++++++++++++++++ internal/openstack/compute_test.go | 57 ++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+) diff --git a/internal/openstack/compute.go b/internal/openstack/compute.go index 5662d40..8686a51 100644 --- a/internal/openstack/compute.go +++ b/internal/openstack/compute.go @@ -9,6 +9,7 @@ import ( "fmt" "io" "net/http" + "slices" "strings" "sync" "time" @@ -127,6 +128,13 @@ func (c *Compute) authenticate(ctx context.Context) (string, string, error) { defer resp.Body.Close() if resp.StatusCode != http.StatusCreated && resp.StatusCode != http.StatusOK { + if methods, ok := supportedMethods(resp.Body); ok && !slices.Contains(methods, applicationCredentialMethod) { + return "", "", fmt.Errorf( + "openstack: this Keystone does not enable the %s authentication method (it offers %s), "+ + "so an application credential cannot be redeemed here however it was created; "+ + "add it to [auth] methods in keystone.conf, or give this agent a credential of a kind it accepts", + applicationCredentialMethod, strings.Join(methods, ", ")) + } // No body: a Keystone error can echo the request, and the request // holds the secret. return "", "", fmt.Errorf("openstack: Keystone refused the application credential (HTTP %d); "+ @@ -166,6 +174,37 @@ func (c *Compute) authenticate(ctx context.Context) (string, string, error) { return tok, endpoint, nil } +// applicationCredentialMethod is the Keystone auth method this client uses. +const applicationCredentialMethod = "application_credential" + +// supportedMethods reads the auth methods a Keystone rejection advertises. +// +// Keystone answers an unsupported method with the list it does support, and +// that list is the one thing in the error body worth reading: it separates "the +// credential is wrong" from "this deployment cannot redeem this kind of +// credential at all", which are different problems with different owners. Only +// the method names are taken — never error.message and never the body — because +// a Keystone error can echo the request, and the request holds the secret. +// +// Found on a CubeCOS cluster, whose keystone.conf ships +// methods = password,token,oauth1,mapped: application credentials can be +// created there and never redeemed, and the old message sent the reader to +// check a credential that was fine. +func supportedMethods(r io.Reader) ([]string, bool) { + var body struct { + Error struct { + Identity struct { + Methods []string `json:"methods"` + } `json:"identity"` + } `json:"error"` + } + if err := json.NewDecoder(io.LimitReader(r, 1<<16)).Decode(&body); err != nil { + return nil, false + } + m := body.Error.Identity.Methods + return m, len(m) > 0 +} + // computeEndpoint picks the compute service URL from the catalog. // // Internal before public: both are on the management network, and the internal diff --git a/internal/openstack/compute_test.go b/internal/openstack/compute_test.go index 9339305..279e111 100644 --- a/internal/openstack/compute_test.go +++ b/internal/openstack/compute_test.go @@ -263,3 +263,60 @@ func TestAKeystoneRefusalNeverQuotesTheSecret(t *testing.T) { t.Errorf("the error carries the secret: %v", err) } } + +// The body is verbatim from a CubeCOS cluster's Keystone (lab validation, +// 2026-09-11), which ships methods = password,token,oauth1,mapped. An +// application credential can be created there and never redeemed; the message +// this replaces sent the reader to check a credential that was fine. +func TestAKeystoneThatCannotRedeemApplicationCredentialsSaysSo(t *testing.T) { + const body = `{"error":{"code":401,"identity":{"methods":["password","token","oauth1","mapped"]},` + + `"message":"Attempted to authenticate with an unsupported method.","title":"Unauthorized"}}` + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(body)) + })) + defer srv.Close() + + c, _ := NewCompute(Credential{ + AuthURL: srv.URL + "/v3", ID: "abc123", Secret: testSecret, Project: "acme-prod", + }) + _, err := c.Post(context.Background(), "/servers", []byte(`{}`), "", 1<<16) + if err == nil { + t.Fatal("a 401 was accepted") + } + msg := err.Error() + if !strings.Contains(msg, "does not enable") { + t.Errorf("the refusal does not name the unsupported method: %v", err) + } + if !strings.Contains(msg, "password") { + t.Errorf("the refusal does not say what this Keystone does accept: %v", err) + } + if strings.Contains(msg, "has not been revoked") { + t.Errorf("the refusal still blames the credential: %v", err) + } + if strings.Contains(msg, testSecret) { + t.Errorf("the error carries the secret: %v", err) + } +} + +// A 401 that is not about the method keeps the original wording: a revoked or +// mistyped credential is still the common case, and this test fails if the new +// branch swallows it. +func TestAnOrdinaryKeystoneRefusalStillBlamesTheCredential(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(`{"error":{"code":401,"title":"Unauthorized"}}`)) + })) + defer srv.Close() + + c, _ := NewCompute(Credential{ + AuthURL: srv.URL + "/v3", ID: "abc123", Secret: testSecret, Project: "acme-prod", + }) + _, err := c.Post(context.Background(), "/servers", []byte(`{}`), "", 1<<16) + if err == nil { + t.Fatal("a 401 was accepted") + } + if !strings.Contains(err.Error(), "has not been revoked") { + t.Errorf("an ordinary 401 lost its wording: %v", err) + } +}