diff --git a/.golangci.toml b/.golangci.toml index 087cc55db..4706fa49c 100644 --- a/.golangci.toml +++ b/.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 diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a6baf3b5..aa7153bb3 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/pkg/login/flow.go b/pkg/login/flow.go index 58600af17..f402368ed 100644 --- a/pkg/login/flow.go +++ b/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() diff --git a/pkg/login/flow_test.go b/pkg/login/flow_test.go index 446df4f5a..fe37db07c 100644 --- a/pkg/login/flow_test.go +++ b/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", + }, + } +}