rebase

Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>

cmd/oras/internal/option/remote.go

@@ -45,6 +45,14 @@ 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 {
@@ -56,7 +64,8 @@ type Remote struct {
 	PasswordFromStdin      bool
 	Password               string
 	IdentityTokenFromStdin bool
-	IdentityToken          string
+	identityToken          string
+	flagPrefix             string
 
 	resolveFlag           []string
 	applyDistributionSpec bool
@@ -75,8 +84,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 from stdin")
-	fs.BoolVarP(&opts.IdentityTokenFromStdin, "identity-token-stdin", "", false, "read identity token from stdin")
+	fs.BoolVarP(&opts.PasswordFromStdin, passwordFromStdinFlag, "", false, "read password from stdin")
+	fs.BoolVarP(&opts.IdentityTokenFromStdin, identityTokenFromStdinFlag, "", false, "read identity token from stdin")
 }
 
 func applyPrefix(prefix, description string) (flagPrefix, notePrefix string) {
@@ -93,61 +102,85 @@ 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")
-	fs.StringVarP(&opts.IdentityToken, flagPrefix+"identity-token", "", "", notePrefix+"registry 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.Password, opts.flagPrefix+passwordFlag, shortPassword, "", notePrefix+"registry password")
+	fs.StringVarP(&opts.identityToken, opts.flagPrefix+identityTokenFlag, "", "", notePrefix+"registry identity token")
+	fs.BoolVarP(&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.StringVarP(&opts.CACertFilePath, opts.flagPrefix+"ca-file", "", "", "server certificate authority file for the remote "+notePrefix+"registry")
+	fs.StringArrayVarP(&opts.resolveFlag, opts.flagPrefix+"resolve", "", nil, "customized DNS for "+notePrefix+"registry, formatted in `host:port:address[:address_port]`")
+	fs.StringArrayVarP(&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 opts.PasswordFromStdin or opts.IdentityTokenFromStdin
+// conflicts with read file from input.
+func (opts *Remote) CheckStdinConflict() error {
+	if opts.PasswordFromStdin {
+		return fmt.Errorf("`-` read file from input and `--%s` read password from input cannot be both used", passwordFromStdinFlag)
+	} else if opts.IdentityTokenFromStdin {
+		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 {
+	// check that basic auth flags and identity token flags are not both used.
+	var flagChecker = func(values []bool, flags []string) string {
+		for i, v := range values {
+			if v {
+				return flags[i]
+			}
+		}
+		return ""
+	}
+	identityTokenFlag := flagChecker([]bool{opts.identityToken != "", opts.IdentityTokenFromStdin},
+		[]string{opts.flagPrefix + identityTokenFlag, identityTokenFromStdinFlag})
+	basicAuthFlag := flagChecker([]bool{opts.Username != "", opts.Password != "", opts.PasswordFromStdin},
+		[]string{opts.flagPrefix + usernameFlag, opts.flagPrefix + passwordFlag, passwordFromStdinFlag})
+
+	if identityTokenFlag != "" && basicAuthFlag != "" {
+		return fmt.Errorf("--%s cannot be used with --%s", basicAuthFlag, identityTokenFlag)
+	}
+
 	if err := opts.parseCustomHeaders(); err != nil {
 		return err
 	}
 	return opts.readPasswordOrIdentityToken()
 }
 
-// readPasswordOrIdentityToken tries to read password and identity token with optional cmd prompt.
+// readPasswordOrIdentityToken tries to read password or identity token with
+// optional cmd prompt.
 func (opts *Remote) readPasswordOrIdentityToken() (err error) {
-	if opts.Password != "" {
-		fmt.Fprintln(os.Stderr, "WARNING! Using --password or --identity-token via the CLI is insecure. Use --password-stdin and --identity-token-stdin.")
-	} else if opts.PasswordFromStdin {
+	if opts.identityToken != "" {
+		fmt.Fprintln(os.Stderr, "WARNING! Using --identity-token via the CLI is insecure. Use --identity-token-stdin.")
+		opts.Password = opts.identityToken
+	} else if opts.Password != "" {
+		fmt.Fprintln(os.Stderr, "WARNING! Using --password via the CLI is insecure. Use --password-stdin.")
+	} else if opts.PasswordFromStdin || opts.IdentityTokenFromStdin {
 		// Prompt for credential
 		password, err := io.ReadAll(os.Stdin)
 		if err != nil {
 			return err
 		}
 		opts.Password = strings.TrimSuffix(string(password), "\n")
 		opts.Password = strings.TrimSuffix(opts.Password, "\r")
-	} else if opts.IdentityTokenFromStdin {
-		// Prompt for credential
-		idToken, err := io.ReadAll(os.Stdin)
-		if err != nil {
-			return err
-		}
-		opts.IdentityToken = strings.TrimSuffix(string(idToken), "\n")
-		opts.IdentityToken = strings.TrimSuffix(opts.IdentityToken, "\r")
 	}
 	return nil
 }

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 := opts.CheckStdinConflict(); err != nil {
+					return err
 				}
 				if opts.size < 0 {
 					return errors.New("`--size` must be provided if the blob is read from stdin")

cmd/oras/root/login.go

@@ -65,21 +65,7 @@ Example - Log in with username and password in an interactive terminal and no TL
 `,
 		Args: oerrors.CheckArgs(argument.Exactly(1), "the registry to log in to"),
 		PreRunE: func(cmd *cobra.Command, args []string) error {
-			if opts.IdentityToken != "" || opts.IdentityTokenFromStdin {
-				if opts.Username != "" {
-					return errors.New("--username cannot be used with --identity-token or --identity-token-stdin")
-				}
-				if opts.Password != "" {
-					return errors.New("--password cannot be used with --identity-token or --identity-token-stdin")
-				}
-			}
-			err := option.Parse(cmd, &opts)
-			if err == nil {
-				if opts.IdentityToken != "" {
-					opts.Password = opts.IdentityToken
-				}
-			}
-			return err
+			return option.Parse(cmd, &opts)
 		},
 		RunE: func(cmd *cobra.Command, args []string) error {
 			opts.Hostname = args[0]

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 := opts.CheckStdinConflict(); err != nil {
+					return err
+				}
 			}
 			refs := strings.Split(args[0], ",")
 			opts.RawReference = refs[0]

test/e2e/suite/auth/auth.go

@@ -100,6 +100,20 @@ 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).
+				MatchErrKeyWords("Error", "--username", "cannot be used with", "--identity-token").
+				ExpectFailure().
+				Exec()
+		})
+
+		It("should fail if password is used with identity token", func() {
+			ORAS("login", ZOTHost, "-p", Password, "--identity-token", Password).
+				MatchErrKeyWords("Error", "--password", "cannot be used with", "--identity-token").
+				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 +167,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() {

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), "-").

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() {