Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: make --identity-token an alias of --password #1294

Merged
merged 9 commits into from
Apr 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@
"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 @@
// 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 @@
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)

Check warning on line 169 in cmd/oras/internal/option/remote.go

View check run for this annotation

Codecov / codecov/patch

cmd/oras/internal/option/remote.go#L169

Added line #L169 was not covered by tests
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")

Check warning on line 174 in cmd/oras/internal/option/remote.go

View check run for this annotation

Codecov / codecov/patch

cmd/oras/internal/option/remote.go#L173-L174

Added lines #L173 - L174 were not covered by tests
}
return nil
}
Expand Down Expand Up @@ -267,7 +303,7 @@

// 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() {
wangxiaoxuan273 marked this conversation as resolved.
Show resolved Hide resolved
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
Loading