feat: Support username and password prompt in login (notaryproject#566)

notaryproject#503

Added e2e tests. The current unit tests don't have RunE() testing code.
Adding the unit test could lead to lots of refactoring. I'm happy to
help the refactoring and start the unit tests of RunE() but I think it
is better to be in a separated PR. Adding lots of unit test refactoring
could mess up this PR.

TODO: Seems login requires credential helper. Need to install credential
helper as a test step.

Signed-off-by: Ziwen Ning <[email protected]>

cmd/notation/login.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"bufio"
 	"context"
 	"errors"
 	"fmt"
@@ -11,6 +12,7 @@ import (
 	"github.com/notaryproject/notation/internal/cmd"
 	"github.com/notaryproject/notation/pkg/auth"
 	"github.com/spf13/cobra"
+	"golang.org/x/term"
 	orasauth "oras.land/oras-go/v2/registry/remote/auth"
 )
 
@@ -65,6 +67,23 @@ func runLogin(ctx context.Context, opts *loginOpts) error {
 	// initialize
 	serverAddress := opts.server
 
+	// input username and password by prompt
+	reader := bufio.NewReader(os.Stdin)
+	var err error
+	if opts.Username == "" {
+		opts.Username, err = readUsernameFromPrompt(reader)
+		if err != nil {
+			return err
+		}
+	}
+
+	if opts.Password == "" {
+		opts.Password, err = readPasswordFromPrompt(reader)
+		if err != nil {
+			return err
+		}
+	}
+
 	if err := validateAuthConfig(ctx, opts, serverAddress); err != nil {
 		return err
 	}
@@ -108,7 +127,7 @@ func newCredentialFromInput(username, password string) orasauth.Credential {
 
 func readPassword(opts *loginOpts) error {
 	if opts.passwordStdin {
-		password, err := readLine()
+		password, err := readLine(os.Stdin)
 		if err != nil {
 			return err
 		}
@@ -117,12 +136,41 @@ func readPassword(opts *loginOpts) error {
 	return nil
 }
 
-func readLine() (string, error) {
-	passwordBytes, err := io.ReadAll(os.Stdin)
+func readLine(r io.Reader) (string, error) {
+	passwordBytes, err := io.ReadAll(r)
 	if err != nil {
 		return "", err
 	}
 	password := strings.TrimSuffix(string(passwordBytes), "\n")
 	password = strings.TrimSuffix(password, "\r")
 	return password, nil
 }
+
+func readUsernameFromPrompt(reader *bufio.Reader) (string, error) {
+	fmt.Print("Username: ")
+	username, err := reader.ReadString('\n')
+	if err != nil {
+		return "", fmt.Errorf("error reading username: %w", err)
+	}
+	username = strings.TrimSpace(username)
+	return username, nil
+}
+
+func readPasswordFromPrompt(reader *bufio.Reader) (string, error) {
+	fmt.Print("Password: ")
+	if term.IsTerminal(int(os.Stdin.Fd())) {
+		bytePassword, err := term.ReadPassword(int(os.Stdin.Fd()))
+		if err != nil {
+			return "", fmt.Errorf("error reading password: %w", err)
+		}
+		fmt.Println()
+		return string(bytePassword), nil
+	} else {
+		password, err := readLine(reader)
+		if err != nil {
+			return "", fmt.Errorf("error reading password: %w", err)
+		}
+		fmt.Println()
+		return password, nil
+	}
+}

go.mod

@@ -11,6 +11,7 @@ require (
 	github.com/sirupsen/logrus v1.9.0
 	github.com/spf13/cobra v1.6.1
 	github.com/spf13/pflag v1.0.5
+	golang.org/x/term v0.5.0
 	oras.land/oras-go/v2 v2.0.2
 )
 

go.sum

@@ -58,6 +58,8 @@ golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBc
 golang.org/x/sys v0.5.0 h1:MUK/U/4lj1t1oPg0HfuXDN/Z1wv31ZJ/YcPiGccS4DU=
 golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
+golang.org/x/term v0.5.0 h1:n2a8QNdAb0sZNpU9R1ALUXBbY+w51fCQDN+7EdxNBsY=
+golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k=
 golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
 golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=

test/e2e/internal/notation/host.go

@@ -75,6 +75,17 @@ func BaseOptions() []utils.HostOption {
 	)
 }
 
+// TestLoginOptions returns the BaseOptions with removing AuthOption and adding ConfigOption.
+// testing environment.
+func TestLoginOptions() []utils.HostOption {
+	return Opts(
+		AddKeyOption("e2e.key", "e2e.crt"),
+		AddTrustStoreOption("e2e", filepath.Join(NotationE2ELocalKeysDir, "e2e.crt")),
+		AddTrustPolicyOption("trustpolicy.json"),
+		AddConfigJsonOption("pass_credential_helper_config.json"),
+	)
+}
+
 // CreateNotationDirOption creates the notation directory in temp user dir.
 func CreateNotationDirOption() utils.HostOption {
 	return func(vhost *utils.VirtualHost) error {
@@ -124,6 +135,16 @@ func AddTrustPolicyOption(trustpolicyName string) utils.HostOption {
 	}
 }
 
+// AddConfigJsonOption adds a valid config.json for testing.
+func AddConfigJsonOption(configJsonName string) utils.HostOption {
+	return func(vhost *utils.VirtualHost) error {
+		return copyFile(
+			filepath.Join(NotationE2EConfigJsonDir, configJsonName),
+			vhost.AbsolutePath(NotationDirName, ConfigJsonName),
+		)
+	}
+}
+
 // AddPlugin adds a pluginkeys.json config file and installs an e2e-plugin.
 func AddPlugin(pluginPath string) utils.HostOption {
 	return func(vhost *utils.VirtualHost) error {

test/e2e/internal/notation/init.go

@@ -16,6 +16,7 @@ const (
 	TrustStoreTypeCA  = "ca"
 	PluginDirName     = "plugins"
 	PluginName        = "e2e-plugin"
+	ConfigJsonName    = "config.json"
 )
 
 const (
@@ -41,6 +42,7 @@ var (
 	NotationE2EConfigPath     string
 	NotationE2ELocalKeysDir   string
 	NotationE2ETrustPolicyDir string
+	NotationE2EConfigJsonDir  string
 )
 
 var (
@@ -78,6 +80,7 @@ func setUpNotationValues() {
 	setPathValue(envKeyNotationConfigPath, &NotationE2EConfigPath)
 	NotationE2ETrustPolicyDir = filepath.Join(NotationE2EConfigPath, "trustpolicies")
 	NotationE2ELocalKeysDir = filepath.Join(NotationE2EConfigPath, LocalKeysDirName)
+	NotationE2EConfigJsonDir = filepath.Join(NotationE2EConfigPath, LocalConfigJsonsDirName)
 }
 
 func setPathValue(envKey string, value *string) {

test/e2e/internal/notation/key.go

@@ -6,8 +6,9 @@ import (
 )
 
 const (
-	SigningKeysFileName = "signingkeys.json"
-	LocalKeysDirName    = "localkeys"
+	SigningKeysFileName     = "signingkeys.json"
+	LocalKeysDirName        = "localkeys"
+	LocalConfigJsonsDirName = "configjsons"
 )
 
 // X509KeyPair contains the paths of a public/private key pair files.

test/e2e/suite/command/login.go

@@ -0,0 +1,40 @@
+package command
+
+import (
+	"fmt"
+
+	. "github.com/notaryproject/notation/test/e2e/internal/notation"
+	"github.com/notaryproject/notation/test/e2e/internal/utils"
+	. "github.com/notaryproject/notation/test/e2e/suite/common"
+	. "github.com/onsi/ginkgo/v2"
+	"github.com/onsi/gomega/gbytes"
+)
+
+var _ = Describe("notation login", func() {
+	BeforeEach(func() {
+		Skip("The login tests require setting up credential helper running in host and it is not available in Github runner. Issue to remove this skip: https://github.com/notaryproject/notation/issues/587")
+	})
+	It("should sign an image after successfully logging in the registry by prompt with a correct credential", func() {
+		Host(TestLoginOptions(), func(notation *utils.ExecOpts, artifact *Artifact, vhost *utils.VirtualHost) {
+			notation.WithInput(gbytes.BufferWithBytes([]byte(fmt.Sprintf("%s\n%s\n", TestRegistry.Username, TestRegistry.Password)))).
+				Exec("login", artifact.Host).
+				MatchKeyWords(LoginSuccessfully)
+			notation.Exec("sign", artifact.ReferenceWithDigest()).
+				MatchKeyWords(SignSuccessfully)
+			notation.Exec("logout", artifact.Host).
+				MatchKeyWords(LogoutSuccessfully)
+		})
+	})
+
+	It("should fail to sign an image after failing to log in the registry with a wrong credential", func() {
+		Host(TestLoginOptions(), func(notation *utils.ExecOpts, artifact *Artifact, vhost *utils.VirtualHost) {
+			notation.WithInput(gbytes.BufferWithBytes([]byte(fmt.Sprintf("%s\n%s\n", "invalidUser", "invalidPassword")))).
+				ExpectFailure().
+				Exec("login", artifact.Host).
+				MatchErrKeyWords("unauthorized")
+			notation.ExpectFailure().
+				Exec("sign", artifact.ReferenceWithDigest()).
+				MatchErrKeyWords("credential required for basic auth")
+		})
+	})
+})

test/e2e/suite/common/common.go

@@ -1,6 +1,8 @@
 package common
 
 const (
+	LoginSuccessfully  = "Login Succeeded"
+	LogoutSuccessfully = "Logout Succeeded"
 	SignSuccessfully   = "Successfully signed"
 	VerifySuccessfully = "Successfully verified"
 	VerifyFailed       = "signature verification failed"

test/e2e/testdata/config/configjsons/pass_credential_helper_config.json

@@ -0,0 +1,3 @@
+{
+    "credsStore": "pass"
+}