diff --git a/pkg/oc/bootstrap/docker/openshift/login.go b/pkg/oc/bootstrap/docker/openshift/login.go index 2d17b8b4f97b..d7dfe00f950a 100644 --- a/pkg/oc/bootstrap/docker/openshift/login.go +++ b/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), } diff --git a/pkg/oc/bootstrap/docker/up.go b/pkg/oc/bootstrap/docker/up.go index 0d554f313534..b72af6f5ecdc 100644 --- a/pkg/oc/bootstrap/docker/up.go +++ b/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 diff --git a/pkg/oc/cli/cli.go b/pkg/oc/cli/cli.go index 68797546c626..a93111926948 100644 --- a/pkg/oc/cli/cli.go +++ b/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{ diff --git a/pkg/oc/cli/cmd/login/login.go b/pkg/oc/cli/cmd/login/login.go index d259ffcbaa86..a139a2a77d6f 100644 --- a/pkg/oc/cli/cmd/login/login.go +++ b/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{ diff --git a/pkg/oc/cli/cmd/login/loginoptions.go b/pkg/oc/cli/cmd/login/loginoptions.go index d047854a5dcc..942f99fed6f8 100644 --- a/pkg/oc/cli/cmd/login/loginoptions.go +++ b/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 diff --git a/pkg/oc/cli/cmd/login/loginoptions_test.go b/pkg/oc/cli/cmd/login/loginoptions_test.go index 1aace2eb283f..0a15a157e092 100644 --- a/pkg/oc/cli/cmd/login/loginoptions_test.go +++ b/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) { diff --git a/test/cmd/login.sh b/test/cmd/login.sh index e9e6fed4df59..e2563f6574e8 100755 --- a/test/cmd/login.sh +++ b/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" diff --git a/test/integration/login_test.go b/test/integration/login_test.go index 6d0985bdcd60..e8d39036e422 100644 --- a/test/integration/login_test.go +++ b/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 diff --git a/test/integration/oauth_oidc_test.go b/test/integration/oauth_oidc_test.go index 5604c7c87dec..772d0b4a6208 100644 --- a/test/integration/oauth_oidc_test.go +++ b/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()) diff --git a/test/integration/oauth_request_header_test.go b/test/integration/oauth_request_header_test.go index fe4fa55b33a4..ad54ecb21904 100644 --- a/test/integration/oauth_request_header_test.go +++ b/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())