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) + } +}