preserve error type in loginoptions

Author: juanvallejo

pkg/oc/bootstrap/docker/openshift/login.go

@@ -18,7 +18,7 @@ import (
 )
 
 // Login logs into the specified server using given credentials and CA file
-func Login(username, password, server, configDir string, f *clientcmd.Factory, c *cobra.Command, out io.Writer) error {
+func Login(username, password, server, configDir string, f *clientcmd.Factory, c *cobra.Command, out, errOut io.Writer) error {
 	existingConfig, err := f.OpenShiftClientConfig().RawConfig()
 	if err != nil {
 		if !os.IsNotExist(err) {
@@ -72,6 +72,7 @@ func Login(username, password, server, configDir string, f *clientcmd.Factory, c
 		Username:           username,
 		Password:           password,
 		Out:                output,
+		ErrOut:             errOut,
 		StartingKubeConfig: newConfig,
 		PathOptions:        config.NewPathOptions(c),
 	}

pkg/oc/bootstrap/docker/up.go

@@ -1112,7 +1112,7 @@ func (c *ClientStartConfig) RegisterTemplateServiceBroker(out io.Writer) error {
 // Login logs into the new server and sets up a default user and project
 func (c *ClientStartConfig) Login(out io.Writer) error {
 	server := c.OpenShiftHelper().Master(c.ServerIP)
-	return openshift.Login(initialUser, initialPassword, server, c.LocalConfigDir, c.originalFactory, c.command, out)
+	return openshift.Login(initialUser, initialPassword, server, c.LocalConfigDir, c.originalFactory, c.command, out, out)
 }
 
 // CreateProject creates a new project for the current user

pkg/oc/cli/cli.go

@@ -86,7 +86,7 @@ func NewCommandCLI(name, fullName string, in io.Reader, out, errout io.Writer) *
 
 	f := clientcmd.New(cmds.PersistentFlags())
 
-	loginCmd := login.NewCmdLogin(fullName, f, in, out)
+	loginCmd := login.NewCmdLogin(fullName, f, in, out, errout)
 	secretcmds := secrets.NewCmdSecrets(secrets.SecretsRecommendedName, fullName+" "+secrets.SecretsRecommendedName, f, in, out, errout, fullName+" edit")
 
 	groups := ktemplates.CommandGroups{

pkg/oc/cli/cmd/login/login.go

@@ -46,10 +46,11 @@ var (
 )
 
 // NewCmdLogin implements the OpenShift cli login command
-func NewCmdLogin(fullName string, f *osclientcmd.Factory, reader io.Reader, out io.Writer) *cobra.Command {
+func NewCmdLogin(fullName string, f *osclientcmd.Factory, reader io.Reader, out, errOut io.Writer) *cobra.Command {
 	options := &LoginOptions{
 		Reader: reader,
 		Out:    out,
+		ErrOut: errOut,
 	}
 
 	cmds := &cobra.Command{

pkg/oc/cli/cmd/login/loginoptions.go

@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"io"
 	"net"
-	"net/http"
 	"os"
 	"path/filepath"
 	"time"
@@ -56,6 +55,7 @@ type LoginOptions struct {
 	Config             *restclient.Config
 	Reader             io.Reader
 	Out                io.Writer
+	ErrOut             io.Writer
 
 	// cert data to be used when authenticating
 	CertFile string
@@ -225,16 +225,9 @@ func (o *LoginOptions) gatherAuthInfo() error {
 	clientConfig.KeyFile = o.KeyFile
 	token, err := tokencmd.RequestToken(o.Config, o.Reader, o.Username, o.Password)
 	if err != nil {
-		suggestion := "verify you have provided the correct host and port and that the server is currently running."
-
-		// if internal error occurs, suggest making sure
-		// client is connecting to the right host:port
-		if statusErr, ok := err.(*kerrors.StatusError); ok {
-			if statusErr.Status().Code == http.StatusInternalServerError {
-				return fmt.Errorf("error: The server was unable to respond - %v", suggestion)
-			}
-		}
-		return fmt.Errorf("%v - %v", err, suggestion)
+		// return error as-is, as method caller expects to check its type
+		fmt.Fprintf(o.ErrOut, "error: %v - %v\n", err, "verify you have provided the correct host and port and that the server is currently running.")
+		return err
 	}
 	clientConfig.BearerToken = token
 

pkg/oc/cli/cmd/login/loginoptions_test.go

@@ -1,8 +1,11 @@
 package login
 
 import (
+	"bytes"
 	"crypto/tls"
+	"encoding/json"
 	"fmt"
+	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
 	"regexp"
@@ -12,12 +15,18 @@ import (
 	"github.com/MakeNowJust/heredoc"
 
 	"github.com/openshift/origin/pkg/cmd/util/clientcmd"
+	"github.com/openshift/origin/pkg/oauth/util"
 	"github.com/openshift/origin/pkg/oc/cli/config"
 
+	kapierrs "k8s.io/apimachinery/pkg/api/errors"
 	restclient "k8s.io/client-go/rest"
 	kclientcmdapi "k8s.io/client-go/tools/clientcmd/api"
 )
 
+const (
+	oauthMetadataEndpoint = "/.well-known/oauth-authorization-server"
+)
+
 func TestNormalizeServerURL(t *testing.T) {
 	testCases := []struct {
 		originalServerURL   string
@@ -256,6 +265,77 @@ func TestDialToHTTPServer(t *testing.T) {
 	}
 }
 
+type oauthMetadataResponse struct {
+	metadata *util.OauthAuthorizationServerMetadata
+}
+
+func (r *oauthMetadataResponse) Serialize() ([]byte, error) {
+	b, err := json.Marshal(r.metadata)
+	if err != nil {
+		return []byte{}, err
+	}
+
+	return b, nil
+}
+
+func TestPreserveErrTypeAuthInfo(t *testing.T) {
+	invoked := make(chan struct{}, 2)
+	oauthResponse := []byte{}
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		select {
+		case invoked <- struct{}{}:
+		default:
+			t.Fatalf("unexpected request handled by test server: %v: %v", r.Method, r.URL)
+		}
+
+		if r.URL.Path == oauthMetadataEndpoint {
+			w.WriteHeader(http.StatusOK)
+			w.Write(oauthResponse)
+			return
+		}
+		w.WriteHeader(http.StatusUnauthorized)
+	}))
+	defer server.Close()
+
+	metadataResponse := &oauthMetadataResponse{}
+	metadataResponse.metadata = &util.OauthAuthorizationServerMetadata{
+		Issuer:                        server.URL,
+		AuthorizationEndpoint:         server.URL + "/oauth/authorize",
+		TokenEndpoint:                 server.URL + "/oauth/token",
+		CodeChallengeMethodsSupported: []string{"plain", "S256"},
+	}
+
+	oauthResponse, err := metadataResponse.Serialize()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	options := &LoginOptions{
+		Server:             server.URL,
+		StartingKubeConfig: &kclientcmdapi.Config{},
+		Username:           "test",
+		Password:           "test",
+		Reader:             bytes.NewReader([]byte{}),
+
+		Config: &restclient.Config{
+			Host: server.URL,
+		},
+
+		Out:    ioutil.Discard,
+		ErrOut: ioutil.Discard,
+	}
+
+	err = options.gatherAuthInfo()
+	if err == nil {
+		t.Fatalf("expecting unauthorized error when gathering authinfo")
+	}
+
+	if !kapierrs.IsUnauthorized(err) {
+		t.Fatalf("expecting error of type metav1.StatusReasonUnauthorized, but got %T", err)
+	}
+}
+
 func TestDialToHTTPSServer(t *testing.T) {
 	invoked := make(chan struct{}, 1)
 	server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

test/cmd/login.sh

@@ -49,6 +49,8 @@ os::cmd::expect_success_and_text "oc login --server=${KUBERNETES_MASTER} --certi
 # make sure standard login prompt is printed once self-provisioner status is restored
 os::cmd::expect_success "oc adm policy add-cluster-role-to-group self-provisioner system:authenticated:oauth --config='${login_kubeconfig}'"
 os::cmd::expect_success_and_text "oc login --server=${KUBERNETES_MASTER} --certificate-authority='${MASTER_CONFIG_DIR}/ca.crt' -u test-user -p anything" "You don't have any projects. You can try to create a new project, by running"
+# make sure `oc login` fails with unauthorized error
+os::cmd::expect_failure_and_text 'oc login <<< \n' 'Login failed \(401 Unauthorized\)'
 os::cmd::expect_success 'oc logout'
 echo "login and status messages: ok"
 

test/integration/login_test.go

@@ -139,7 +139,8 @@ func newLoginOptions(server string, username string, password string, insecure b
 		Password:           password,
 		InsecureTLS:        insecure,
 
-		Out: ioutil.Discard,
+		Out:    ioutil.Discard,
+		ErrOut: ioutil.Discard,
 	}
 
 	return loginOptions

test/integration/oauth_oidc_test.go

@@ -162,6 +162,7 @@ func TestOAuthOIDC(t *testing.T) {
 		StartingKubeConfig: &clientcmdapi.Config{},
 		Reader:             bytes.NewBufferString("mylogin\nmypassword\n"),
 		Out:                loginOutput,
+		ErrOut:             ioutil.Discard,
 	}
 	if err := loginOptions.GatherInfo(); err != nil {
 		t.Fatalf("Error logging in: %v\n%v", err, loginOutput.String())

test/integration/oauth_request_header_test.go

@@ -310,6 +310,7 @@ func TestOAuthRequestHeader(t *testing.T) {
 		StartingKubeConfig: &clientcmdapi.Config{},
 		Reader:             bytes.NewBufferString("myusername\nmypassword\n"),
 		Out:                loginOutput,
+		ErrOut:             ioutil.Discard,
 	}
 	if err := loginOptions.GatherInfo(); err != nil {
 		t.Fatalf("Error trying to determine server info: %v\n%v", err, loginOutput.String())