diff --git a/cmd/oras/internal/option/remote.go b/cmd/oras/internal/option/remote.go index 9620292bf..abe24ce3d 100644 --- a/cmd/oras/internal/option/remote.go +++ b/cmd/oras/internal/option/remote.go @@ -45,16 +45,25 @@ import ( "oras.land/oras/internal/version" ) +const ( + usernameFlag = "username" + passwordFlag = "password" + passwordFromStdinFlag = "password-stdin" + identityTokenFlag = "identity-token" + identityTokenFromStdinFlag = "identity-token-stdin" +) + // Remote options struct contains flags and arguments specifying one registry. // Remote implements oerrors.Handler and interface. type Remote struct { DistributionSpec - CACertFilePath string - Insecure bool - Configs []string - Username string - PasswordFromStdin bool - Password string + CACertFilePath string + Insecure bool + Configs []string + Username string + secretFromStdin bool + Secret string + flagPrefix string resolveFlag []string applyDistributionSpec bool @@ -73,7 +82,8 @@ func (opts *Remote) EnableDistributionSpecFlag() { // ApplyFlags applies flags to a command flag set. func (opts *Remote) ApplyFlags(fs *pflag.FlagSet) { opts.ApplyFlagsWithPrefix(fs, "", "") - fs.BoolVarP(&opts.PasswordFromStdin, "password-stdin", "", false, "read password or identity token from stdin") + fs.BoolVar(&opts.secretFromStdin, passwordFromStdinFlag, false, "read password from stdin") + fs.BoolVar(&opts.secretFromStdin, identityTokenFromStdinFlag, false, "read identity token from stdin") } func applyPrefix(prefix, description string) (flagPrefix, notePrefix string) { @@ -90,52 +100,78 @@ func (opts *Remote) ApplyFlagsWithPrefix(fs *pflag.FlagSet, prefix, description shortUser string shortPassword string shortHeader string - flagPrefix string notePrefix string ) if prefix == "" { shortUser, shortPassword = "u", "p" shortHeader = "H" } - flagPrefix, notePrefix = applyPrefix(prefix, description) + opts.flagPrefix, notePrefix = applyPrefix(prefix, description) if opts.applyDistributionSpec { opts.DistributionSpec.ApplyFlagsWithPrefix(fs, prefix, description) } - fs.StringVarP(&opts.Username, flagPrefix+"username", shortUser, "", notePrefix+"registry username") - fs.StringVarP(&opts.Password, flagPrefix+"password", shortPassword, "", notePrefix+"registry password or identity token") - fs.BoolVarP(&opts.Insecure, flagPrefix+"insecure", "", false, "allow connections to "+notePrefix+"SSL registry without certs") - plainHTTPFlagName := flagPrefix + "plain-http" + fs.StringVarP(&opts.Username, opts.flagPrefix+usernameFlag, shortUser, "", notePrefix+"registry username") + fs.StringVarP(&opts.Secret, opts.flagPrefix+passwordFlag, shortPassword, "", notePrefix+"registry password or identity token") + fs.StringVar(&opts.Secret, opts.flagPrefix+identityTokenFlag, "", notePrefix+"registry identity token") + fs.BoolVar(&opts.Insecure, opts.flagPrefix+"insecure", false, "allow connections to "+notePrefix+"SSL registry without certs") + plainHTTPFlagName := opts.flagPrefix + "plain-http" plainHTTP := fs.Bool(plainHTTPFlagName, false, "allow insecure connections to "+notePrefix+"registry without SSL check") opts.plainHTTP = func() (bool, bool) { return *plainHTTP, fs.Changed(plainHTTPFlagName) } - fs.StringVarP(&opts.CACertFilePath, flagPrefix+"ca-file", "", "", "server certificate authority file for the remote "+notePrefix+"registry") - fs.StringArrayVarP(&opts.resolveFlag, flagPrefix+"resolve", "", nil, "customized DNS for "+notePrefix+"registry, formatted in `host:port:address[:address_port]`") - fs.StringArrayVarP(&opts.Configs, flagPrefix+"registry-config", "", nil, "`path` of the authentication file for "+notePrefix+"registry") - fs.StringArrayVarP(&opts.headerFlags, flagPrefix+"header", shortHeader, nil, "add custom headers to "+notePrefix+"requests") + fs.StringVar(&opts.CACertFilePath, opts.flagPrefix+"ca-file", "", "server certificate authority file for the remote "+notePrefix+"registry") + fs.StringArrayVar(&opts.resolveFlag, opts.flagPrefix+"resolve", nil, "customized DNS for "+notePrefix+"registry, formatted in `host:port:address[:address_port]`") + fs.StringArrayVar(&opts.Configs, opts.flagPrefix+"registry-config", nil, "`path` of the authentication file for "+notePrefix+"registry") + fs.StringArrayVarP(&opts.headerFlags, opts.flagPrefix+"header", shortHeader, nil, "add custom headers to "+notePrefix+"requests") +} + +// CheckStdinConflict checks if PasswordFromStdin or IdentityTokenFromStdin of a +// *pflag.FlagSet conflicts with read file from input. +func CheckStdinConflict(flags *pflag.FlagSet) error { + switch { + case flags.Changed(passwordFromStdinFlag): + return fmt.Errorf("`-` read file from input and `--%s` read password from input cannot be both used", passwordFromStdinFlag) + case flags.Changed(identityTokenFromStdinFlag): + return fmt.Errorf("`-` read file from input and `--%s` read identity token from input cannot be both used", identityTokenFromStdinFlag) + } + return nil } // Parse tries to read password with optional cmd prompt. -func (opts *Remote) Parse(*cobra.Command) error { +func (opts *Remote) Parse(cmd *cobra.Command) error { + usernameAndIdTokenFlags := []string{opts.flagPrefix + usernameFlag, opts.flagPrefix + identityTokenFlag} + passwordAndIdTokenFlags := []string{opts.flagPrefix + passwordFlag, opts.flagPrefix + identityTokenFlag} + if cmd.Flags().Lookup(identityTokenFromStdinFlag) != nil { + usernameAndIdTokenFlags = append(usernameAndIdTokenFlags, identityTokenFromStdinFlag) + passwordAndIdTokenFlags = append(passwordAndIdTokenFlags, identityTokenFromStdinFlag) + } + if cmd.Flags().Lookup(passwordFromStdinFlag) != nil { + passwordAndIdTokenFlags = append(passwordAndIdTokenFlags, passwordFromStdinFlag) + } + cmd.MarkFlagsMutuallyExclusive(usernameAndIdTokenFlags...) + cmd.MarkFlagsMutuallyExclusive(passwordAndIdTokenFlags...) if err := opts.parseCustomHeaders(); err != nil { return err } - return opts.readPassword() + return opts.readSecret(cmd) } -// readPassword tries to read password with optional cmd prompt. -func (opts *Remote) readPassword() (err error) { - if opts.Password != "" { +// readSecret tries to read password or identity token with +// optional cmd prompt. +func (opts *Remote) readSecret(cmd *cobra.Command) (err error) { + if cmd.Flags().Changed(identityTokenFlag) { + fmt.Fprintln(os.Stderr, "WARNING! Using --identity-token via the CLI is insecure. Use --identity-token-stdin.") + } else if cmd.Flags().Changed(passwordFlag) { fmt.Fprintln(os.Stderr, "WARNING! Using --password via the CLI is insecure. Use --password-stdin.") - } else if opts.PasswordFromStdin { + } else if opts.secretFromStdin { // Prompt for credential - password, err := io.ReadAll(os.Stdin) + secret, err := io.ReadAll(os.Stdin) if err != nil { return err } - opts.Password = strings.TrimSuffix(string(password), "\n") - opts.Password = strings.TrimSuffix(opts.Password, "\r") + opts.Secret = strings.TrimSuffix(string(secret), "\n") + opts.Secret = strings.TrimSuffix(opts.Secret, "\r") } return nil } @@ -267,7 +303,7 @@ func (opts *Remote) parseCustomHeaders() error { // Credential returns a credential based on the remote options. func (opts *Remote) Credential() auth.Credential { - return credential.Credential(opts.Username, opts.Password) + return credential.Credential(opts.Username, opts.Secret) } func (opts *Remote) handleWarning(registry string, logger logrus.FieldLogger) func(warning remote.Warning) { diff --git a/cmd/oras/internal/option/remote_test.go b/cmd/oras/internal/option/remote_test.go index a55a4edc3..bc5a071f2 100644 --- a/cmd/oras/internal/option/remote_test.go +++ b/cmd/oras/internal/option/remote_test.go @@ -80,7 +80,7 @@ func TestRemote_authClient_RawCredential(t *testing.T) { } opts := Remote{ Username: want.Username, - Password: want.Password, + Secret: want.Password, } client, err := opts.authClient("hostname", false) if err != nil { diff --git a/cmd/oras/internal/option/target_test.go b/cmd/oras/internal/option/target_test.go index 7c9a33917..9a227b768 100644 --- a/cmd/oras/internal/option/target_test.go +++ b/cmd/oras/internal/option/target_test.go @@ -44,7 +44,9 @@ func TestTarget_Parse_remote(t *testing.T) { RawReference: "mocked/test", IsOCILayout: false, } - if err := opts.Parse(nil); err != nil { + cmd := &cobra.Command{} + ApplyFlags(&opts, cmd.Flags()) + if err := opts.Parse(cmd); err != nil { t.Errorf("Target.Parse() error = %v", err) } if opts.Type != TargetTypeRemote { diff --git a/cmd/oras/root/blob/push.go b/cmd/oras/root/blob/push.go index 5d562e50f..b399600c3 100644 --- a/cmd/oras/root/blob/push.go +++ b/cmd/oras/root/blob/push.go @@ -80,8 +80,8 @@ Example - Push blob 'hi.txt' into an OCI image layout folder 'layout-dir': opts.RawReference = args[0] opts.fileRef = args[1] if opts.fileRef == "-" { - if opts.PasswordFromStdin { - return errors.New("`-` read file from input and `--password-stdin` read password from input cannot be both used") + if err := option.CheckStdinConflict(cmd.Flags()); err != nil { + return err } if opts.size < 0 { return errors.New("`--size` must be provided if the blob is read from stdin") diff --git a/cmd/oras/root/login.go b/cmd/oras/root/login.go index 3535d1a3f..be34b13ef 100644 --- a/cmd/oras/root/login.go +++ b/cmd/oras/root/login.go @@ -52,10 +52,10 @@ Example - Log in with username and password from stdin: oras login -u username --password-stdin localhost:5000 Example - Log in with identity token from command line flags: - oras login -p token localhost:5000 + oras login --identity-token token localhost:5000 Example - Log in with identity token from stdin: - oras login --password-stdin localhost:5000 + oras login --identity-token-stdin localhost:5000 Example - Log in with username and password in an interactive terminal: oras login localhost:5000 @@ -81,7 +81,7 @@ func runLogin(cmd *cobra.Command, opts loginOptions) (err error) { outWriter := cmd.OutOrStdout() // prompt for credential - if opts.Password == "" { + if opts.Secret == "" { if opts.Username == "" { // prompt for username username, err := readLine(outWriter, "Username: ", false) @@ -92,16 +92,16 @@ func runLogin(cmd *cobra.Command, opts loginOptions) (err error) { } if opts.Username == "" { // prompt for token - if opts.Password, err = readLine(outWriter, "Token: ", true); err != nil { + if opts.Secret, err = readLine(outWriter, "Token: ", true); err != nil { return err - } else if opts.Password == "" { + } else if opts.Secret == "" { return errors.New("token required") } } else { // prompt for password - if opts.Password, err = readLine(outWriter, "Password: ", true); err != nil { + if opts.Secret, err = readLine(outWriter, "Password: ", true); err != nil { return err - } else if opts.Password == "" { + } else if opts.Secret == "" { return errors.New("password required") } } diff --git a/cmd/oras/root/manifest/push.go b/cmd/oras/root/manifest/push.go index facaf7ce0..c59eeccfd 100644 --- a/cmd/oras/root/manifest/push.go +++ b/cmd/oras/root/manifest/push.go @@ -86,8 +86,10 @@ Example - Push a manifest to an OCI image layout folder 'layout-dir' and tag wit Args: oerrors.CheckArgs(argument.Exactly(2), "the destination to push to and the file to read manifest content from"), PreRunE: func(cmd *cobra.Command, args []string) error { opts.fileRef = args[1] - if opts.fileRef == "-" && opts.PasswordFromStdin { - return errors.New("`-` read file from input and `--password-stdin` read password from input cannot be both used") + if opts.fileRef == "-" { + if err := option.CheckStdinConflict(cmd.Flags()); err != nil { + return err + } } refs := strings.Split(args[0], ",") opts.RawReference = refs[0] diff --git a/test/e2e/suite/auth/auth.go b/test/e2e/suite/auth/auth.go index a31c7fec6..38c5df2b5 100644 --- a/test/e2e/suite/auth/auth.go +++ b/test/e2e/suite/auth/auth.go @@ -100,6 +100,14 @@ var _ = Describe("Common registry user", func() { Expect(err).Should(gbytes.Say(`Run "oras login -h"`)) }) + It("should fail if username is used with identity token", func() { + ORAS("login", ZOTHost, "-u", Username, "--identity-token", Password).ExpectFailure().Exec() + }) + + It("should fail if password is used with identity token", func() { + ORAS("login", ZOTHost, "-p", Password, "--identity-token", Password).ExpectFailure().Exec() + }) + It("should fail if no username input", func() { ORAS("login", ZOTHost, "--registry-config", filepath.Join(GinkgoT().TempDir(), tmpConfigName)). WithTimeOut(20 * time.Second). @@ -153,6 +161,11 @@ var _ = Describe("Common registry user", func() { WithInput(strings.NewReader(fmt.Sprintf("%s\n%s\n", Username, Password))). MatchKeyWords("Username: ", "Password: ", "Login Succeeded\n").Exec() }) + + It("should fail as the test server doesn't support token service", func() { + ORAS("login", ZOTHost, "--identity-token", Password). + MatchErrKeyWords("WARNING", "Using --identity-token via the CLI is insecure", "Use --identity-token-stdin").ExpectFailure().Exec() + }) }) When("using legacy config", func() { diff --git a/test/e2e/suite/command/blob.go b/test/e2e/suite/command/blob.go index 3ca5cfafa..717a7d366 100644 --- a/test/e2e/suite/command/blob.go +++ b/test/e2e/suite/command/blob.go @@ -45,6 +45,14 @@ var _ = Describe("ORAS beginners:", func() { ExpectFailure(). MatchErrKeyWords("Error: `-` read file from input and `--password-stdin` read password from input cannot be both used").Exec() }) + + It("should fail to read blob content and identity token from stdin at the same time", func() { + repo := fmt.Sprintf(repoFmt, "push", "password-stdin") + ORAS("blob", "push", RegistryRef(ZOTHost, repo, ""), "--identity-token-stdin", "-"). + ExpectFailure(). + MatchErrKeyWords("Error: `-` read file from input and `--identity-token-stdin` read identity token from input cannot be both used").Exec() + }) + It("should fail to push a blob from stdin but no blob size provided", func() { repo := fmt.Sprintf(repoFmt, "push", "no-size") ORAS("blob", "push", RegistryRef(ZOTHost, repo, pushDigest), "-"). diff --git a/test/e2e/suite/command/cp.go b/test/e2e/suite/command/cp.go index 11683c51e..42ca1ff97 100644 --- a/test/e2e/suite/command/cp.go +++ b/test/e2e/suite/command/cp.go @@ -99,6 +99,12 @@ var _ = Describe("ORAS beginners:", func() { ORAS("cp", src, dst, "--from-username", Username, "--from-password", Password+"?"). MatchErrKeyWords(RegistryErrorPrefix).ExpectFailure().Exec() }) + + It("should fail if basic auth flag is used with identity token flag", func() { + src := RegistryRef(ZOTHost, cpTestRepo("conflicted-flags"), foobar.Tag) + dst := RegistryRef(ZOTHost, ArtifactRepo, "") + ORAS("cp", src, dst, "--from-username", Username, "--from-identity-token", "test-token").ExpectFailure().Exec() + }) }) }) diff --git a/test/e2e/suite/command/manifest.go b/test/e2e/suite/command/manifest.go index bf644acb1..f72f3a959 100644 --- a/test/e2e/suite/command/manifest.go +++ b/test/e2e/suite/command/manifest.go @@ -71,12 +71,19 @@ var _ = Describe("ORAS beginners:", func() { gomega.Expect(err).Should(gbytes.Say(`Run "oras manifest push -h"`)) }) - It("should fail pushing with a manifest from stdin without media type flag", func() { + It("should fail pushing with a manifest from stdin with password read from stdin", func() { tag := "from-stdin" ORAS("manifest", "push", RegistryRef(ZOTHost, ImageRepo, tag), "-", "--password-stdin", "--media-type", "application/vnd.oci.image.manifest.v1+json"). ExpectFailure(). MatchErrKeyWords("`-`", "`--password-stdin`", " cannot be both used").Exec() }) + + It("should fail pushing with a manifest from stdin with identity token read from stdin", func() { + tag := "from-stdin" + ORAS("manifest", "push", RegistryRef(ZOTHost, ImageRepo, tag), "-", "--identity-token-stdin", "--media-type", "application/vnd.oci.image.manifest.v1+json"). + ExpectFailure(). + MatchErrKeyWords("`-`", "`--identity-token-stdin`", " cannot be both used").Exec() + }) }) When("running `manifest fetch`", func() {