Fix nil pointer dereference

This fixes a regression introduced in #1502, which can cause a
panic on non-desktop environments with browser-based login flows
other than auth0.

https://jira.mesosphere.com/browse/DCOS-60349

Author: bamarni

.golangci.toml

@@ -35,7 +35,7 @@ enable = [
 line-length = 150
 
 [linters-settings.gocyclo]
-min-complexity = 22
+min-complexity = 23
 
 [linters-settings.govet]
 check-shadowing = false

CHANGELOG.md

@@ -2,6 +2,12 @@
 
 ## Next
 
+## 1.1.1
+
+* Fixes
+
+  * Fix panic on non-desktop environments with browser login flows other than auth0
+
 ## 1.1.0
 
 * Features

pkg/login/flow.go

@@ -185,7 +185,9 @@ func (f *Flow) triggerMethod(provider *Provider) (acsToken string, err error) {
 					// Falling back to reading from STDIN is safer here.
 					//
 					// See https://jira.mesosphere.com/browse/DCOS_OSS-5591
-					loginServer.Close()
+					if loginServer != nil {
+						loginServer.Close()
+					}
 				} else if loginServer != nil {
 					token = <-loginServer.Token()
 

pkg/login/flow_test.go

@@ -97,34 +97,49 @@ func TestTriggerMethodOIDCWithLoginServer(t *testing.T) {
 
 func TestTriggerMethodOIDCWithoutLoginServer(t *testing.T) {
 
-	// We're simulating a container environment where the browser can't be opened.
-	// See https://jira.mesosphere.com/browse/DCOS_OSS-5591
-	opener := open.OpenerFunc(func(_ string) error {
-		return errors.New("couldn't open browser, I'm just a container")
-	})
+	testCases := []struct {
+		provider *Provider
+	}{
+		{
+			// We're simulating a container environment where the browser can't be opened.
+			// See https://jira.mesosphere.com/browse/DCOS_OSS-5591
+			defaultOIDCImplicitFlowProvider(),
+		},
+		{
+			// Non-regression test for a panic on browser login flows other than auth0.
+			// https://jira.mesosphere.com/browse/DCOS-60349
+			shibbolethLoginProvider(),
+		},
+	}
 
-	expectedLoginToken := "dummy_login_token"
-	expectedACSToken := "dummy_acs_token"
+	for _, tc := range testCases {
+		expectedLoginToken := "dummy_login_token"
+		expectedACSToken := "dummy_acs_token"
 
-	ts := httptest.NewServer(mockLoginEndpoint(t, expectedLoginToken, expectedACSToken))
-	defer ts.Close()
+		ts := httptest.NewServer(mockLoginEndpoint(t, expectedLoginToken, expectedACSToken))
+		defer ts.Close()
 
-	logger := &logrus.Logger{Out: ioutil.Discard}
+		logger := &logrus.Logger{Out: ioutil.Discard}
 
-	var in bytes.Buffer
-	in.WriteString(expectedLoginToken)
+		var in bytes.Buffer
+		in.WriteString(expectedLoginToken)
 
-	flow := NewFlow(FlowOpts{
-		Prompt: prompt.New(&in, ioutil.Discard),
-		Logger: logger,
-		Opener: opener,
-	})
+		opener := open.OpenerFunc(func(_ string) error {
+			return errors.New("couldn't open browser, I'm just a container")
+		})
 
-	flow.client = NewClient(httpclient.New(ts.URL), logger)
+		flow := NewFlow(FlowOpts{
+			Prompt: prompt.New(&in, ioutil.Discard),
+			Logger: logger,
+			Opener: opener,
+		})
 
-	acsToken, err := flow.triggerMethod(defaultOIDCImplicitFlowProvider())
-	require.NoError(t, err)
-	require.Equal(t, expectedACSToken, acsToken)
+		flow.client = NewClient(httpclient.New(ts.URL), logger)
+
+		acsToken, err := flow.triggerMethod(tc.provider)
+		require.NoError(t, err)
+		require.Equal(t, expectedACSToken, acsToken)
+	}
 }
 
 func mockLoginEndpoint(t *testing.T, expectedLoginToken, acsToken string) http.Handler {
@@ -148,3 +163,14 @@ func mockLoginEndpoint(t *testing.T, expectedLoginToken, acsToken string) http.H
 	})
 	return mux
 }
+
+func shibbolethLoginProvider() *Provider {
+	return &Provider{
+		ID:           "shib-integration-test",
+		Type:         OIDCImplicitFlow,
+		ClientMethod: methodBrowserOIDCToken,
+		Config: ProviderConfig{
+			StartFlowURL: "/login?redirect_uri=urn:ietf:wg:oauth:2.0:oob",
+		},
+	}
+}