From cb49ca4ba4c6d4ea8b46a4ffd9bcf931ec73ea53 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 14 Sep 2026 12:21:17 +0000 Subject: [PATCH] feat(auth): read the password from stdin or prompt for it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --os-password / OS_PASSWORD were the only ways in, and neither is a good place for a secret: a flag value is visible in `ps` and lands in the shell history, and an exported variable is inherited by every child process. Two more sources, both consulted from Authenticate: - --os-password-stdin reads it from standard input, `docker login --password-stdin` style: the whole of stdin less one trailing line ending. koc-native. More than one line is rejected, since that is a whole openrc piped in by mistake and authenticating with its first line is the confusing outcome. It conflicts with an explicitly typed --os-password and with --creds-from-ns / --creds-from-vault, overrides OS_PASSWORD, and outranks a named cloud's stored password the way a typed flag does. - An interactive prompt when nothing else supplied one and stdin is a terminal — python-openstackclient parity, which osc-lib does through getpass — including for a clouds.yaml entry that deliberately stores no password. A non-interactive run still fails with "no credentials found" rather than blocking on a pipe nobody will write to. Neither fires when the request authenticates without a password (application credentials, a pre-issued token), and both are remembered on Options so a second authentication in the same process does not ask twice. Exercised end to end against a mock Keystone: the piped password and one typed at the prompt over a pty each reach /v3/auth/tokens, --debug still redacts them, and the conflict, empty-stdin and multi-line-stdin cases all fail before any request is made. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01McdyrP2uagmAnTsE3oMvha --- README.md | 34 +++ docs/coverage.md | 20 +- internal/auth/credsfrom.go | 2 +- internal/auth/flagnames.go | 1 + internal/auth/options.go | 22 +- internal/auth/password.go | 181 ++++++++++++++ internal/auth/password_test.go | 437 +++++++++++++++++++++++++++++++++ internal/auth/provider.go | 19 +- 8 files changed, 705 insertions(+), 11 deletions(-) create mode 100644 internal/auth/password.go create mode 100644 internal/auth/password_test.go diff --git a/README.md b/README.md index fe76103..3151556 100644 --- a/README.md +++ b/README.md @@ -194,6 +194,40 @@ so `koc baremetal driver show ipmi --os-system-scope all` works from a shell that already has a project-scoped openrc sourced. `all` is the only value Keystone defines. +#### Where the password comes from + +`--os-password` / `OS_PASSWORD` is the usual answer, but neither is a good place +for a secret: a flag value is visible in `ps` and lands in the shell history, and +an environment variable is inherited by every child process. Two more ways in: + +| Source | Use it for | +| --- | --- | +| `--os-password-stdin` | scripts and CI — `koc … --os-password-stdin < secret` | +| the interactive prompt | a shell session, and a `clouds.yaml` entry that deliberately stores no password | + +`--os-password-stdin` follows `docker login --password-stdin`: `koc` reads +standard input, strips one trailing line ending, and uses the rest verbatim +(leading and trailing spaces included — only the newline goes). More than one +line is an error, since that is a whole openrc piped in by mistake rather than a +password. It conflicts with an explicitly typed `--os-password` and with +`--creds-from-ns` / `--creds-from-vault`, which bring their own credentials; it +overrides `OS_PASSWORD`, which is background configuration, and it outranks a +named cloud's stored password the same way a typed `--os-password` does. + +```sh +koc server list --os-password-stdin < ~/.config/koc/password +pass show keystack/admin | koc server list --os-cloud keystack --os-password-stdin +``` + +When nothing supplies a password and the run is interactive, `koc` asks for it on +the terminal without echo, the way `python-openstackclient` does — including for +a named cloud whose `clouds.yaml` entry has a `username` but no `password`. A +non-interactive run is never prompted: it fails with `no credentials found` +instead of blocking on a pipe nobody is going to write to. Neither source is +consulted when the request authenticates without a password (application +credentials, a pre-issued token), and `--os-password-stdin` is not combined with +a prompt — stdin has already been spent. + #### Alternative credential sources Two koc-specific, mutually exclusive flags source credentials outside the normal diff --git a/docs/coverage.md b/docs/coverage.md index 7961a6d..005b3b5 100644 --- a/docs/coverage.md +++ b/docs/coverage.md @@ -458,12 +458,20 @@ so the gap is upstream's, not the cloud's. The write side is already reachable: `nova clear-password` is `koc server set --no-password`, and changing the password is `koc server set --password`. -One **global flag** is koc-native too, and is deliberately not in the counts -above (the tables measure commands, not flags): `--timeout` / `OS_TIMEOUT` caps a -whole HTTP request/response exchange on every client `koc` builds. OSC has no -global equivalent — keystoneauth carries a session `timeout`, but -`python-openstackclient` registers no flag for it, so an operator's only recourse -upstream is `clouds.yaml`. See README "Timeouts" for the semantics. +Two **global flags** are koc-native too, and are deliberately not in the counts +above (the tables measure commands, not flags): + +- `--timeout` / `OS_TIMEOUT` caps a whole HTTP request/response exchange on every + client `koc` builds. OSC has no global equivalent — keystoneauth carries a + session `timeout`, but `python-openstackclient` registers no flag for it, so an + operator's only recourse upstream is `clouds.yaml`. See README "Timeouts" for + the semantics. +- `--os-password-stdin` reads the password from standard input. Upstream has no + equivalent: osc-lib's only non-`OS_PASSWORD` route is the interactive `getpass` + prompt, which a CI job cannot use, leaving `--os-password` (visible in `ps` and + the shell history) or an exported `OS_PASSWORD` (inherited by every child + process). `koc` keeps that prompt — it is the parity half of the same change — + and adds the pipe. See README "Where the password comes from". `port list --all-projects` is koc-native for the same reason and likewise not counted: neutron has no cross-project switch, because an admin token already diff --git a/internal/auth/credsfrom.go b/internal/auth/credsfrom.go index f338bdf..7d478b3 100644 --- a/internal/auth/credsfrom.go +++ b/internal/auth/credsfrom.go @@ -622,7 +622,7 @@ func (o *Options) applyOpenrcVars(kv map[string]string) { set("os-auth-url", &o.AuthURL, "OS_AUTH_URL") set("os-username", &o.Username, "OS_USERNAME") set("os-user-id", &o.UserID, "OS_USER_ID") - set("os-password", &o.Password, "OS_PASSWORD") + set(flagOSPassword, &o.Password, "OS_PASSWORD") set(flagOSProjectName, &o.ProjectName, "OS_PROJECT_NAME", "OS_TENANT_NAME") set(flagOSProjectID, &o.ProjectID, "OS_PROJECT_ID", "OS_TENANT_ID") set("os-project-domain-name", &o.ProjectDomainName, "OS_PROJECT_DOMAIN_NAME") diff --git a/internal/auth/flagnames.go b/internal/auth/flagnames.go index c255a9e..4ba4868 100644 --- a/internal/auth/flagnames.go +++ b/internal/auth/flagnames.go @@ -8,4 +8,5 @@ package auth const ( flagOSProjectName = "os-project-name" flagOSProjectID = "os-project-id" + flagOSPassword = "os-password" ) diff --git a/internal/auth/options.go b/internal/auth/options.go index 3cbd3a7..0005bc3 100644 --- a/internal/auth/options.go +++ b/internal/auth/options.go @@ -11,6 +11,11 @@ // 4. Application credentials (OS_APPLICATION_CREDENTIAL_ID / _SECRET), // which are honored through either of the two paths above. // +// The password has two further sources, both in password.go and neither a +// separate precedence tier: --os-password-stdin (koc-native) reads it from +// standard input instead of a flag or the environment, and a run that reaches +// authentication without one is prompted on the terminal, as osc-lib does. +// // Naming a cloud selects it wholesale: because every auth flag defaults to its // OS_* variable, a sourced openrc would otherwise override the named cloud // field by field and silently send the command — credentials included — to the @@ -26,6 +31,7 @@ package auth import ( "context" "fmt" + "io" "os" "strconv" "strings" @@ -65,6 +71,7 @@ type Options struct { Username string UserID string Password string + PasswordStdin bool ProjectName string ProjectID string ProjectDomainName string @@ -161,6 +168,12 @@ type Options struct { // see testhooks.go for why the hook is here rather than in an // export_test.go. authenticate func(context.Context) (*Client, error) + + // passwordStdinSrc and promptPassword seam the two terminal password + // sources (see password.go). Nil in every non-test build, meaning the real + // os.Stdin and an unechoed read from it. + passwordStdinSrc io.Reader + promptPassword func(io.Writer) (string, error) } // markForced records that flag's value was supplied by a source pflag cannot @@ -234,8 +247,15 @@ func (o *Options) AddFlags(fs *pflag.FlagSet) { "username (env OS_USERNAME)") fs.StringVar(&o.UserID, "os-user-id", os.Getenv("OS_USER_ID"), "user ID (env OS_USER_ID)") - fs.StringVar(&o.Password, "os-password", os.Getenv("OS_PASSWORD"), + fs.StringVar(&o.Password, flagOSPassword, os.Getenv("OS_PASSWORD"), "password (env OS_PASSWORD)") + // UNVERIFIED against KeyStack: koc-native, python-openstackclient has no + // equivalent (it only prompts). Deliberately flag-only — reading a secret + // from stdin is a thing a run must opt into visibly, not something an + // exported variable can turn on under a command that wanted stdin for + // something else. + fs.BoolVar(&o.PasswordStdin, flagOSPasswordStdin, false, + "read the password from standard input instead of --os-password, which is visible in ps and the shell history") fs.StringVar(&o.ProjectName, flagOSProjectName, os.Getenv("OS_PROJECT_NAME"), "project name (env OS_PROJECT_NAME)") fs.StringVar(&o.ProjectID, flagOSProjectID, os.Getenv("OS_PROJECT_ID"), diff --git a/internal/auth/password.go b/internal/auth/password.go new file mode 100644 index 0000000..f7a46d0 --- /dev/null +++ b/internal/auth/password.go @@ -0,0 +1,181 @@ +package auth + +import ( + "errors" + "fmt" + "io" + "os" + "strings" + + "github.com/gophercloud/gophercloud/v2" + "golang.org/x/term" +) + +// Two ways to supply the Keystone password other than --os-password / +// OS_PASSWORD, both of which exist because that pair is a poor place for a +// secret: a flag value is visible in `ps` and lands in the shell history, and +// an environment variable is inherited by every child process. +// +// - --os-password-stdin reads the password from standard input. It is +// koc-native — python-openstackclient has no equivalent — and follows +// `docker login --password-stdin`: koc reads stdin, strips one trailing +// line ending, and uses the rest verbatim. +// - The interactive prompt is python-openstackclient parity. osc-lib asks on +// the terminal (getpass) when the chosen auth type needs a password and +// nothing supplied one, rather than failing; koc now does the same. +// +// koc prompts only when stdin is a terminal. A non-interactive run must fail +// with the usual "no credentials found" error instead of blocking forever on a +// pipe nobody is going to write to. + +const flagOSPasswordStdin = "os-password-stdin" + +// passwordPrompt matches what osc-lib writes, so an operator moving between the +// two clients sees the same line. It goes to stderr: stdout may be a redirected +// -f json document. +const passwordPrompt = "Password: " + +// applyPasswordStdin consumes stdin into o.Password when --os-password-stdin is +// set. It is called before any credential is used, so a conflicting source is +// reported before the first network round trip. +func (o *Options) applyPasswordStdin() error { + if !o.PasswordStdin || o.forced[flagOSPassword] { + // Already read. Authenticate can run more than once in a process, and + // stdin can only be consumed once, so the first read stands. + return nil + } + switch { + case o.Password != "" && o.explicitlySet(flagOSPassword): + return fmt.Errorf("--%s and --os-password are mutually exclusive", flagOSPasswordStdin) + case o.CredsFromNS != "": + return fmt.Errorf("--%s cannot be combined with --creds-from-ns, which brings its own credentials", flagOSPasswordStdin) + case o.CredsFromVault != "": + return fmt.Errorf("--%s cannot be combined with --creds-from-vault, which brings its own credentials", flagOSPasswordStdin) + } + + pw, err := readPasswordStdin(o.stdin()) + if err != nil { + return err + } + o.rememberPassword(pw) + return nil +} + +// readPasswordStdin takes the whole of r as the password, less one trailing +// line ending. +func readPasswordStdin(r io.Reader) (string, error) { + raw, err := io.ReadAll(r) + if err != nil { + return "", fmt.Errorf("--%s: reading stdin: %w", flagOSPasswordStdin, err) + } + // Only the line ending goes: a password may legitimately begin or end with + // a space, and `echo`, a here-doc and every text editor append a newline. + pw := strings.TrimSuffix(string(raw), "\n") + pw = strings.TrimSuffix(pw, "\r") + + switch { + case pw == "": + return "", fmt.Errorf("--%s: no password on stdin", flagOSPasswordStdin) + case strings.ContainsAny(pw, "\r\n"): + // Almost always a whole openrc or secrets file piped in by mistake. + // Authenticating with the first line and failing is the confusing + // outcome; say what happened instead. + return "", fmt.Errorf("--%s: stdin holds more than one line, so it is not just a password", flagOSPasswordStdin) + } + return pw, nil +} + +// stdin is the reader --os-password-stdin consumes, seamed for tests. +func (o *Options) stdin() io.Reader { + if o.passwordStdinSrc != nil { + return o.passwordStdinSrc + } + return os.Stdin +} + +// promptMissingPassword asks for the password on the terminal when the resolved +// auth options need one and no source produced it. With no terminal to ask on +// it changes nothing: the caller's own "no credentials" error is the better +// message, and gophercloud rejects the request either way. +func (o *Options) promptMissingPassword(ao *gophercloud.AuthOptions, w io.Writer) error { + if !needsPassword(ao) { + return nil + } + ask := o.terminalPassword() + if ask == nil { + return nil + } + pw, err := ask(w) + if err != nil { + return err + } + if pw == "" { + return errors.New("no password given at the prompt") + } + ao.Password = pw + o.rememberPassword(pw) + return nil +} + +// rememberPassword records a password that reached koc from stdin or the +// terminal as though --os-password had carried it — it is as deliberate as one +// typed on the command line, so a named cloud's stored password must not +// outrank it, and a second authentication in the same process +// must not ask for it again (gophercloud's own reauth replays the auth options +// and never gets here). +func (o *Options) rememberPassword(pw string) { + o.Password = pw + o.markForced(flagOSPassword) +} + +// needsPassword reports whether ao describes password authentication for a +// named user with the password still missing. Application credentials and a +// pre-issued token authenticate without one, so neither is prompted for. +func needsPassword(ao *gophercloud.AuthOptions) bool { + if ao.Password != "" || ao.TokenID != "" { + return false + } + if ao.ApplicationCredentialID != "" || ao.ApplicationCredentialName != "" { + return false + } + return ao.Username != "" || ao.UserID != "" +} + +// willPromptForPassword reports whether a missing password will be asked for +// rather than rejected, for the env path's up-front credential check — which +// runs before the auth options exist and so tests o's own fields. +func (o *Options) willPromptForPassword() bool { + if o.Username == "" && o.UserID == "" { + return false + } + return o.terminalPassword() != nil +} + +// terminalPassword returns the function that asks for a password, or nil when +// there is no terminal to ask on. +func (o *Options) terminalPassword() func(io.Writer) (string, error) { + if o.promptPassword != nil { + return o.promptPassword + } + if o.PasswordStdin || !term.IsTerminal(int(os.Stdin.Fd())) { + return nil + } + return readTerminalPassword +} + +// readTerminalPassword prompts on w and reads stdin without echo. +func readTerminalPassword(w io.Writer) (string, error) { + if _, err := fmt.Fprint(w, passwordPrompt); err != nil { + return "", err + } + pw, err := term.ReadPassword(int(os.Stdin.Fd())) + // The Enter the operator typed was swallowed with the echo, so the next + // thing written to the terminal would otherwise land on the prompt line. + if _, perr := fmt.Fprintln(w); perr != nil && err == nil { + err = perr + } + if err != nil { + return "", fmt.Errorf("reading the password: %w", err) + } + return string(pw), nil +} diff --git a/internal/auth/password_test.go b/internal/auth/password_test.go new file mode 100644 index 0000000..5996c03 --- /dev/null +++ b/internal/auth/password_test.go @@ -0,0 +1,437 @@ +package auth + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "io" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/gophercloud/gophercloud/v2" + "github.com/spf13/pflag" +) + +// --- --os-password-stdin --- + +func TestReadPasswordStdin(t *testing.T) { + cases := []struct { + name string + in string + want string + wantErr string + }{ + {name: "bare", in: "s3cret", want: "s3cret"}, + {name: "trailing newline", in: "s3cret\n", want: "s3cret"}, + {name: "trailing CRLF", in: "s3cret\r\n", want: "s3cret"}, + // A password may legitimately end in a space; only the line ending goes. + {name: "keeps surrounding spaces", in: " s3 cret \n", want: " s3 cret "}, + {name: "keeps inner quoting", in: `"s3cret"` + "\n", want: `"s3cret"`}, + {name: "empty", in: "", wantErr: "no password on stdin"}, + {name: "newline only", in: "\n", wantErr: "no password on stdin"}, + {name: "whole openrc", in: "export OS_PASSWORD=s3cret\nexport OS_USERNAME=admin\n", wantErr: "more than one line"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, err := readPasswordStdin(strings.NewReader(tc.in)) + if tc.wantErr != "" { + if err == nil || !strings.Contains(err.Error(), tc.wantErr) { + t.Fatalf("error = %v, want one containing %q", err, tc.wantErr) + } + return + } + if err != nil { + t.Fatalf("readPasswordStdin: %v", err) + } + if got != tc.want { + t.Errorf("password = %q, want %q", got, tc.want) + } + }) + } +} + +func TestReadPasswordStdin_ReadError(t *testing.T) { + _, err := readPasswordStdin(errReader{}) + if err == nil || !strings.Contains(err.Error(), "reading stdin") { + t.Fatalf("error = %v, want a read failure", err) + } +} + +type errReader struct{} + +func (errReader) Read([]byte) (int, error) { return 0, errors.New("boom") } + +func TestApplyPasswordStdin_PopulatesPasswordAndOutranksACloud(t *testing.T) { + o := &Options{PasswordStdin: true, passwordStdinSrc: strings.NewReader("piped\n")} + if err := o.applyPasswordStdin(); err != nil { + t.Fatalf("applyPasswordStdin: %v", err) + } + if o.Password != "piped" { + t.Errorf("password = %q, want piped", o.Password) + } + // A piped password is as deliberate as a typed one, so it must survive the + // clouds.yaml guard in Options.override. + o.Cloud = "named" + if got := o.override("os-password", o.Password); got != "piped" { + t.Errorf("override dropped the piped password: %q", got) + } +} + +func TestApplyPasswordStdin_Inert(t *testing.T) { + o := &Options{Password: "fromenv", passwordStdinSrc: strings.NewReader("piped\n")} + if err := o.applyPasswordStdin(); err != nil { + t.Fatalf("applyPasswordStdin: %v", err) + } + if o.Password != "fromenv" { + t.Errorf("password = %q, want the flag/env value untouched", o.Password) + } +} + +// An explicitly typed --os-password and --os-password-stdin name two different +// secrets; picking one silently is how the wrong credential reaches Keystone. +func TestApplyPasswordStdin_ConflictsWithExplicitFlag(t *testing.T) { + o := &Options{} + fs := pflag.NewFlagSet("koc", pflag.ContinueOnError) + o.AddFlags(fs) + if err := fs.Parse([]string{"--os-password", "typed", "--os-password-stdin"}); err != nil { + t.Fatalf("parse: %v", err) + } + o.passwordStdinSrc = strings.NewReader("piped\n") + + err := o.applyPasswordStdin() + if err == nil || !strings.Contains(err.Error(), "mutually exclusive") { + t.Fatalf("error = %v, want a mutual-exclusion error", err) + } +} + +// OS_PASSWORD is background configuration, not a typed flag, so --os-password- +// stdin overrides it rather than erroring. +func TestApplyPasswordStdin_BeatsEnvPassword(t *testing.T) { + t.Setenv("OS_PASSWORD", "fromenv") + o := &Options{} + fs := pflag.NewFlagSet("koc", pflag.ContinueOnError) + o.AddFlags(fs) + if err := fs.Parse([]string{"--os-password-stdin"}); err != nil { + t.Fatalf("parse: %v", err) + } + o.passwordStdinSrc = strings.NewReader("piped\n") + + if err := o.applyPasswordStdin(); err != nil { + t.Fatalf("applyPasswordStdin: %v", err) + } + if o.Password != "piped" { + t.Errorf("password = %q, want piped", o.Password) + } +} + +func TestApplyPasswordStdin_ConflictsWithCredsFrom(t *testing.T) { + for _, tc := range []struct { + name, want string + opts Options + }{ + {name: "ns", want: "--creds-from-ns", opts: Options{CredsFromNS: "ironic"}}, + {name: "vault", want: "--creds-from-vault", opts: Options{CredsFromVault: "openrc"}}, + } { + t.Run(tc.name, func(t *testing.T) { + o := tc.opts + o.PasswordStdin = true + o.passwordStdinSrc = strings.NewReader("piped\n") + err := o.applyPasswordStdin() + if err == nil || !strings.Contains(err.Error(), tc.want) { + t.Fatalf("error = %v, want one naming %s", err, tc.want) + } + }) + } +} + +// --os-password-stdin has taken stdin, so there is nothing left to prompt on +// even when the process still has a terminal. +func TestTerminalPassword_NilWhenStdinIsThePassword(t *testing.T) { + o := &Options{PasswordStdin: true} + if o.terminalPassword() != nil { + t.Error("--os-password-stdin must not also prompt") + } +} + +// --- the interactive prompt --- + +func TestPromptMissingPassword_AsksAndFills(t *testing.T) { + var w bytes.Buffer + o := &Options{promptPassword: func(out io.Writer) (string, error) { + _, _ = io.WriteString(out, passwordPrompt) + return "typed", nil + }} + ao := gophercloud.AuthOptions{Username: "admin"} + + if err := o.promptMissingPassword(&ao, &w); err != nil { + t.Fatalf("promptMissingPassword: %v", err) + } + if ao.Password != "typed" { + t.Errorf("password = %q, want typed", ao.Password) + } + if w.String() != passwordPrompt { + t.Errorf("prompt = %q, want %q", w.String(), passwordPrompt) + } +} + +func TestPromptMissingPassword_EmptyAnswerIsAnError(t *testing.T) { + o := &Options{promptPassword: func(io.Writer) (string, error) { return "", nil }} + ao := gophercloud.AuthOptions{Username: "admin"} + if err := o.promptMissingPassword(&ao, io.Discard); err == nil { + t.Fatal("expected an error when the operator just hit Enter") + } +} + +func TestPromptMissingPassword_PropagatesReadError(t *testing.T) { + o := &Options{promptPassword: func(io.Writer) (string, error) { return "", errors.New("no tty") }} + ao := gophercloud.AuthOptions{Username: "admin"} + if err := o.promptMissingPassword(&ao, io.Discard); err == nil { + t.Fatal("expected the terminal read error to propagate") + } +} + +// Everything that authenticates without a password must go through untouched — +// prompting for one would be a lie about what the request needs. +func TestPromptMissingPassword_SkipsWhenNoPasswordIsNeeded(t *testing.T) { + cases := map[string]gophercloud.AuthOptions{ + "password already set": {Username: "admin", Password: "s3cret"}, + "application credential": {ApplicationCredentialID: "ac1", ApplicationCredentialSecret: "s"}, + "app credential by name": {Username: "admin", ApplicationCredentialName: "deploy"}, + "pre-issued token": {TokenID: "gAAAAA"}, + "no user to ask about": {IdentityEndpoint: "https://keystone.example/v3"}, + } + for name, ao := range cases { + t.Run(name, func(t *testing.T) { + o := &Options{promptPassword: func(io.Writer) (string, error) { + t.Error("must not prompt") + return "", nil + }} + before := ao + if err := o.promptMissingPassword(&ao, io.Discard); err != nil { + t.Fatalf("promptMissingPassword: %v", err) + } + if ao != before { + t.Errorf("auth options were modified: %+v", ao) + } + }) + } +} + +// With no terminal the env path's own error is the better message, so the +// prompt step must leave the options alone rather than invent a failure. +func TestPromptMissingPassword_NoTerminalIsNotAnError(t *testing.T) { + o := &Options{} // no seam, and `go test` stdin is not a terminal + ao := gophercloud.AuthOptions{Username: "admin"} + if err := o.promptMissingPassword(&ao, io.Discard); err != nil { + t.Fatalf("promptMissingPassword: %v", err) + } + if ao.Password != "" { + t.Errorf("password = %q, want it left empty", ao.Password) + } +} + +// The env path rejects a missing password up front — unless it is about to be +// asked for. +func TestResolveAuth_EnvMissingPasswordDefersToThePrompt(t *testing.T) { + o := &Options{ + AuthURL: "https://keystone.example/v3", + Username: "admin", + promptPassword: func(io.Writer) (string, error) { + return "typed", nil + }, + } + if _, _, _, err := o.resolveAuth(); err != nil { + t.Fatalf("resolveAuth must defer to the prompt: %v", err) + } + + // ...but only when there is a user to prompt about. + anon := &Options{AuthURL: "https://keystone.example/v3", promptPassword: o.promptPassword} + if _, _, _, err := anon.resolveAuth(); err == nil { + t.Error("expected the no-credentials error when there is no user to prompt about") + } +} + +// --- end to end against a mock Keystone --- + +// passwordCapture answers a token request and records the password Keystone was +// asked to verify. +func passwordCapture(t *testing.T) (*httptest.Server, *string) { + t.Helper() + var got string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var body struct { + Auth struct { + Identity struct { + Password struct { + User struct { + Password string `json:"password"` + } `json:"user"` + } `json:"password"` + } `json:"identity"` + } `json:"auth"` + } + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + t.Errorf("decoding the token request: %v", err) + } + got = body.Auth.Identity.Password.User.Password + w.Header().Set("Content-Type", "application/json") + w.Header().Set("X-Subject-Token", "gAAAAAtoken") + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte(tokenResponse)) + })) + t.Cleanup(srv.Close) + return srv, &got +} + +func TestAuthenticate_PasswordFromStdin(t *testing.T) { + srv, got := passwordCapture(t) + o := mockOptions(srv.URL + "/v3") + o.Password = "" + o.PasswordStdin = true + o.passwordStdinSrc = strings.NewReader("piped-password\n") + + if _, err := o.Authenticate(context.Background()); err != nil { + t.Fatalf("Authenticate: %v", err) + } + if *got != "piped-password" { + t.Errorf("keystone verified %q, want piped-password", *got) + } +} + +func TestAuthenticate_PromptsForAMissingPassword(t *testing.T) { + srv, got := passwordCapture(t) + o := mockOptions(srv.URL + "/v3") + o.Password = "" + asked := 0 + o.promptPassword = func(io.Writer) (string, error) { + asked++ + return "typed-password", nil + } + + if _, err := o.Authenticate(context.Background()); err != nil { + t.Fatalf("Authenticate: %v", err) + } + if asked != 1 { + t.Errorf("prompted %d times, want 1", asked) + } + if *got != "typed-password" { + t.Errorf("keystone verified %q, want typed-password", *got) + } +} + +// The password the operator typed has to be kept for gophercloud's reauth, or +// the second token request of a long command asks for it again. +func TestAuthenticate_PromptedPasswordSurvivesForReauth(t *testing.T) { + srv, got := passwordCapture(t) + o := mockOptions(srv.URL + "/v3") + o.Password = "" + prompted := 0 + o.promptPassword = func(io.Writer) (string, error) { + prompted++ + return "typed-password", nil + } + + client, err := o.Authenticate(context.Background()) + if err != nil { + t.Fatalf("Authenticate: %v", err) + } + if client.Provider.ReauthFunc == nil { + t.Fatal("provider has no reauth function") + } + + *got = "" + if err := client.Provider.ReauthFunc(context.Background()); err != nil { + t.Fatalf("reauth: %v", err) + } + if *got != "typed-password" { + t.Errorf("reauth sent %q, want the password typed at the first prompt", *got) + } + if prompted != 1 { + t.Errorf("prompted %d times, want 1 — reauth must replay the answer, not ask again", prompted) + } +} + +func TestAuthenticate_StdinConflictReportedBeforeAnyRequest(t *testing.T) { + srv, got := passwordCapture(t) + o := mockOptions(srv.URL + "/v3") + o.PasswordStdin = true + o.passwordStdinSrc = strings.NewReader("piped\n") + o.CredsFromVault = "openrc" + + if _, err := o.Authenticate(context.Background()); err == nil { + t.Fatal("expected a mutual-exclusion error") + } + if *got != "" { + t.Errorf("keystone was contacted with %q", *got) + } +} + +// The classic osc-lib case: a clouds.yaml entry that deliberately stores no +// password, so the operator is asked for it at each invocation. +func TestAuthenticate_PromptsForACloudWithNoStoredPassword(t *testing.T) { + srv, got := passwordCapture(t) + cloudsPath := filepath.Join(t.TempDir(), "clouds.yaml") + yaml := "clouds:\n keystack:\n auth:\n auth_url: " + srv.URL + "/v3\n" + + " username: admin\n project_name: admin\n" + + " user_domain_name: Default\n project_domain_name: Default\n" + if err := os.WriteFile(cloudsPath, []byte(yaml), 0o600); err != nil { + t.Fatal(err) + } + t.Setenv("OS_CLIENT_CONFIG_FILE", cloudsPath) + + o := &Options{Cloud: "keystack", Timeout: 10 * time.Second} + o.promptPassword = func(io.Writer) (string, error) { return "typed-password", nil } + + if _, err := o.Authenticate(context.Background()); err != nil { + t.Fatalf("Authenticate: %v", err) + } + if *got != "typed-password" { + t.Errorf("keystone verified %q, want typed-password", *got) + } +} + +// stdin can only be consumed once, and a prompt must not reappear, so a second +// authentication in the same process reuses what the first one obtained. +func TestPasswordSourcesAreConsultedOnce(t *testing.T) { + t.Run("stdin", func(t *testing.T) { + src := strings.NewReader("piped\n") + o := &Options{PasswordStdin: true, passwordStdinSrc: src} + if err := o.applyPasswordStdin(); err != nil { + t.Fatalf("first read: %v", err) + } + if err := o.applyPasswordStdin(); err != nil { + t.Fatalf("second read: %v", err) // an exhausted reader would error + } + if o.Password != "piped" { + t.Errorf("password = %q, want piped", o.Password) + } + }) + + t.Run("prompt", func(t *testing.T) { + srv, got := passwordCapture(t) + o := mockOptions(srv.URL + "/v3") + o.Password = "" + asked := 0 + o.promptPassword = func(io.Writer) (string, error) { + asked++ + return "typed-password", nil + } + for i := range 2 { + if _, err := o.Authenticate(context.Background()); err != nil { + t.Fatalf("Authenticate %d: %v", i, err) + } + } + if asked != 1 { + t.Errorf("prompted %d times, want 1", asked) + } + if *got != "typed-password" { + t.Errorf("keystone verified %q on the second run", *got) + } + }) +} diff --git a/internal/auth/provider.go b/internal/auth/provider.go index 813ac15..c7dbc22 100644 --- a/internal/auth/provider.go +++ b/internal/auth/provider.go @@ -4,6 +4,7 @@ import ( "context" "crypto/tls" "fmt" + "os" "github.com/gophercloud/gophercloud/v2" "github.com/gophercloud/gophercloud/v2/openstack/config" @@ -41,6 +42,12 @@ func (o *Options) Authenticate(ctx context.Context) (*Client, error) { if err := o.validateSystemScope(); err != nil { return nil, err } + // --os-password-stdin is read here, before the first network round trip, so + // a run that pipes its password in fails on a conflicting source rather than + // on a rejected token request. + if err := o.applyPasswordStdin(); err != nil { + return nil, err + } // Vault: fetch an openrc secret and fold its OS_* values into o, then fall // through to the normal Keystone flow below (works for every service). @@ -64,6 +71,12 @@ func (o *Options) Authenticate(ctx context.Context) (*Client, error) { if err != nil { return nil, err } + // Nothing supplied a password: ask for it the way osc-lib does, on the + // terminal. AllowReauth below then replays whatever was typed, so a token + // expiring mid-command does not ask twice. + if err := o.promptMissingPassword(&ao, os.Stderr); err != nil { + return nil, err + } ao.AllowReauth = true tlsCfg, insecure, err := o.resolveTLSConfig(baseTLS) @@ -115,8 +128,8 @@ func (o *Options) resolveAuth() (gophercloud.AuthOptions, gophercloud.EndpointOp if o.AuthURL == "" { return ao, eo, nil, fmt.Errorf("no credentials found: set --os-cloud, or OS_AUTH_URL and the related OS_* variables") } - if o.Password == "" && o.AppCredID == "" && o.AppCredName == "" { - return ao, eo, nil, fmt.Errorf("no credentials found: set OS_PASSWORD or application credentials (OS_APPLICATION_CREDENTIAL_ID/_SECRET)") + if o.Password == "" && o.AppCredID == "" && o.AppCredName == "" && !o.willPromptForPassword() { + return ao, eo, nil, fmt.Errorf("no credentials found: set OS_PASSWORD or application credentials (OS_APPLICATION_CREDENTIAL_ID/_SECRET), pass --os-password-stdin, or run interactively to be prompted") } eo = gophercloud.EndpointOpts{Region: o.RegionName} } @@ -138,7 +151,7 @@ func (o *Options) applyAuthOverrides(ao *gophercloud.AuthOptions) { setIf(&ao.IdentityEndpoint, o.override("os-auth-url", o.AuthURL)) setIf(&ao.Username, o.override("os-username", o.Username)) setIf(&ao.UserID, o.override("os-user-id", o.UserID)) - setIf(&ao.Password, o.override("os-password", o.Password)) + setIf(&ao.Password, o.override(flagOSPassword, o.Password)) setIf(&ao.TenantName, o.override(flagOSProjectName, o.ProjectName)) setIf(&ao.TenantID, o.override(flagOSProjectID, o.ProjectID)) setIf(&ao.ApplicationCredentialID, o.override("os-application-credential-id", o.AppCredID))