Skip to content

Commit

Permalink
feat: make --identity-token an alias of --password (oras-project#1294)
Browse files Browse the repository at this point in the history
Signed-off-by: Xiaoxuan Wang <[email protected]>
  • Loading branch information
wangxiaoxuan273 authored Apr 12, 2024
1 parent bb3443d commit 61557e0
Show file tree
Hide file tree
Showing 10 changed files with 115 additions and 41 deletions.
90 changes: 63 additions & 27 deletions cmd/oras/internal/option/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand All @@ -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
}
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion cmd/oras/internal/option/remote_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 3 additions & 1 deletion cmd/oras/internal/option/target_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions cmd/oras/root/blob/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
14 changes: 7 additions & 7 deletions cmd/oras/root/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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")
}
}
Expand Down
6 changes: 4 additions & 2 deletions cmd/oras/root/manifest/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
13 changes: 13 additions & 0 deletions test/e2e/suite/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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() {
Expand Down
8 changes: 8 additions & 0 deletions test/e2e/suite/command/blob.go
Original file line number Diff line number Diff line change
Expand Up @@ -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), "-").
Expand Down
6 changes: 6 additions & 0 deletions test/e2e/suite/command/cp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
})
})

Expand Down
9 changes: 8 additions & 1 deletion test/e2e/suite/command/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down

0 comments on commit 61557e0

Please sign in to comment.