Make linter happy by deleting unused gocyclo

Fix form_post.js mistake from recent commit; Better CORS on callback
(cherry picked from commit 5d79d4b9dc)
2026-01-28 16:42:22 +00:00 · 2022-02-09 10:38:28 -08:00 · 2022-02-09 10:26:24 -08:00 · 2022-02-09 10:21:23 -08:00 · 2022-02-09 10:21:23 -08:00 · 2022-02-09 10:21:23 -08:00
12 changed files with 541 additions and 169 deletions
--- a/internal/oidc/provider/formposthtml/form_post.js
+++ b/internal/oidc/provider/formposthtml/form_post.js
@@ -1,4 +1,4 @@
-// Copyright 2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0

 window.onload = () => {
@@ -44,11 +44,22 @@ window.onload = () => {
        responseParams['redirect_uri'].value,
        {
            method: 'POST',
-            mode: 'no-cors',
+            mode: 'no-cors', // in the future, we could change this to "cors" (see comment below)
            headers: {'Content-Type': 'application/x-www-form-urlencoded;charset=UTF-8'},
            body: responseParams['encoded_params'].value,
        })
-        .then(() => clearTimeout(timeout))
-        .then(() => transitionToState('success'))
+        .then(response => {
+            clearTimeout(timeout);
+            // Requests made using "no-cors" mode will hide the real response.status by making it 0
+            // and the real response.ok by making it false.
+            // If the real response was success, then we would like to show the success state.
+            // If the real response was an error, then we wish we could show the manual
+            // state, but we have no way to know that, as long as we are making "no-cors" requests.
+            // For now, show the success status for all responses.
+            // In the future, we could make this request in "cors" mode once old versions of our CLI
+            // which did not handle CORS are upgraded out by our users. That would allow us to use
+            // a conditional statement based on response.ok here to decide which state to transition into.
+            transitionToState('success');
+        })
        .catch(() => transitionToState('manual'));
 };
--- a/internal/oidc/provider/formposthtml/formposthtml_test.go
+++ b/internal/oidc/provider/formposthtml/formposthtml_test.go
@@ -1,4 +1,4 @@
-// Copyright 2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0

 package formposthtml
@@ -30,7 +30,7 @@ var (
        <head>
            <meta charset="UTF-8">
            <style>body{font-family:metropolis-light,Helvetica,sans-serif}h1{font-size:20px}.state{position:absolute;top:100px;left:50%;width:400px;height:80px;margin-top:-40px;margin-left:-200px;font-size:14px;line-height:24px}button{margin:-10px;padding:10px;text-align:left;width:100%;display:inline;border:none;background:0 0;cursor:pointer;transition:all .1s}button:hover{background-color:#eee;transform:scale(1.01)}button:active{background-color:#ddd;transform:scale(.99)}code{display:block;word-wrap:break-word;word-break:break-all;font-size:12px;font-family:monospace;color:#333}.copy-icon{float:left;width:36px;height:36px;margin-top:-3px;margin-right:10px;background-size:contain;background-repeat:no-repeat;background-image:url("data:image/svg+xml,%3Csvg width=%2236%22 height=%2236%22 viewBox=%220 0 36 36%22 xmlns=%22http://www.w3.org/2000/svg%22 xmlns:xlink=%22http://www.w3.org/1999/xlink%22%3E%3Ctitle%3Ecopy-to-clipboard-line%3C/title%3E%3Cpath d=%22M22.6 4H21.55a3.89 3.89.0 00-7.31.0H13.4A2.41 2.41.0 0011 6.4V10H25V6.4A2.41 2.41.0 0022.6 4zM23 8H13V6.25A.25.25.0 0113.25 6h2.69l.12-1.11A1.24 1.24.0 0116.61 4a2 2 0 013.15 1.18l.09.84h2.9a.25.25.0 01.25.25z%22 class=%22clr-i-outline clr-i-outline-path-1%22/%3E%3Cpath d=%22M33.25 18.06H21.33l2.84-2.83a1 1 0 10-1.42-1.42L17.5 19.06l5.25 5.25a1 1 0 00.71.29 1 1 0 00.71-1.7l-2.84-2.84H33.25a1 1 0 000-2z%22 class=%22clr-i-outline clr-i-outline-path-2%22/%3E%3Cpath d=%22M29 16h2V6.68A1.66 1.66.0 0029.35 5H27.08V7H29z%22 class=%22clr-i-outline clr-i-outline-path-3%22/%3E%3Cpath d=%22M29 31H7V7H9V5H6.64A1.66 1.66.0 005 6.67V31.32A1.66 1.66.0 006.65 33H29.36A1.66 1.66.0 0031 31.33V22.06H29z%22 class=%22clr-i-outline clr-i-outline-path-4%22/%3E%3Crect x=%220%22 y=%220%22 width=%2236%22 height=%2236%22 fill-opacity=%220%22/%3E%3C/svg%3E")}@keyframes loader{to{transform:rotate(360deg)}}#loading{content:'';box-sizing:border-box;width:80px;height:80px;margin-top:-40px;margin-left:-40px;border-radius:50%;border:2px solid #fff;border-top-color:#1b3951;animation:loader .6s linear infinite}</style>
-            <script>window.onload=()=>{const a=b=>{Array.from(document.querySelectorAll('.state')).forEach(a=>a.hidden=!0);const a=document.getElementById(b);a.hidden=!1,document.title=a.dataset.title,document.getElementById('favicon').setAttribute('href','data:image/svg+xml,<svg xmlns=%22http://www.w3.org/2000/svg%22 viewBox=%220 0 100 100%22><text y=%22.9em%22 font-size=%2290%22>'+a.dataset.favicon+'</text></svg>')};a('loading'),window.history.replaceState(null,'','./'),document.getElementById('manual-copy-button').onclick=()=>{const a=document.getElementById('manual-copy-button').innerText;navigator.clipboard.writeText(a).then(()=>console.info('copied authorization code '+a+' to clipboard')).catch(b=>console.error('failed to copy code '+a+' to clipboard: '+b))};const c=setTimeout(()=>a('manual'),2e3),b=document.forms[0].elements;fetch(b.redirect_uri.value,{method:'POST',mode:'no-cors',headers:{'Content-Type':'application/x-www-form-urlencoded;charset=UTF-8'},body:b.encoded_params.value}).then(()=>clearTimeout(c)).then(()=>a('success')).catch(()=>a('manual'))}</script>
+            <script>window.onload=()=>{const a=b=>{Array.from(document.querySelectorAll('.state')).forEach(a=>a.hidden=!0);const a=document.getElementById(b);a.hidden=!1,document.title=a.dataset.title,document.getElementById('favicon').setAttribute('href','data:image/svg+xml,<svg xmlns=%22http://www.w3.org/2000/svg%22 viewBox=%220 0 100 100%22><text y=%22.9em%22 font-size=%2290%22>'+a.dataset.favicon+'</text></svg>')};a('loading'),window.history.replaceState(null,'','./'),document.getElementById('manual-copy-button').onclick=()=>{const a=document.getElementById('manual-copy-button').innerText;navigator.clipboard.writeText(a).then(()=>console.info('copied authorization code '+a+' to clipboard')).catch(b=>console.error('failed to copy code '+a+' to clipboard: '+b))};const c=setTimeout(()=>a('manual'),2e3),b=document.forms[0].elements;fetch(b.redirect_uri.value,{method:'POST',mode:'no-cors',headers:{'Content-Type':'application/x-www-form-urlencoded;charset=UTF-8'},body:b.encoded_params.value}).then(b=>{clearTimeout(c),a('success')}).catch(()=>a('manual'))}</script>
            <link id="favicon" rel="icon"/>
        </head>
        <body>
@@ -61,7 +61,7 @@ var (
 	// It's okay if this changes in the future, but this gives us a chance to eyeball the formatting.
 	// Our browser-based integration tests should find any incompatibilities.
 	testExpectedCSP = `default-src 'none'; ` +
-		`script-src 'sha256-U+tKnJ2oMSYKSxmSX3V2mPBN8xdr9JpampKAhbSo108='; ` +
+		`script-src 'sha256-+M/LwI0kltqjqTbsYcEYpN4nMkcCMkOmJcr1pbUSP2Q='; ` +
 		`style-src 'sha256-CtfkX7m8x2UdGYvGgDq+6b6yIAQsASW9pbQK+sG8fNA='; ` +
 		`img-src data:; ` +
 		`connect-src *; ` +
@@ -83,6 +83,7 @@ func TestTemplate(t *testing.T) {
 		Parameters: testResponseParams,
 	}))

+	// t.Logf("actual value:\n%s", buf2.String()) // useful when updating minify library causes new output
 	require.Equal(t, buf.String(), buf2.String())
 	require.Equal(t, testExpectedFormPostOutput, buf.String())
 }
--- a/internal/testutil/kube_server_compatibility.go
+++ b/internal/testutil/kube_server_compatibility.go
@@ -0,0 +1,30 @@
+// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package testutil
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	certificatesv1 "k8s.io/api/certificates/v1"
+	"k8s.io/client-go/discovery"
+)
+
+func KubeServerSupportsCertificatesV1API(t *testing.T, discoveryClient discovery.DiscoveryInterface) bool {
+	t.Helper()
+	groupList, err := discoveryClient.ServerGroups()
+	require.NoError(t, err)
+	for _, group := range groupList.Groups {
+		if group.Name == certificatesv1.GroupName {
+			for _, version := range group.Versions {
+				if version.Version == "v1" {
+					// Note: v1 should exist in Kubernetes 1.19 and above
+					return true
+				}
+			}
+		}
+		continue
+	}
+	return false
+}
--- a/pkg/oidcclient/login.go
+++ b/pkg/oidcclient/login.go
@@ -1,4 +1,4 @@
-// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0

 // Package oidcclient implements a CLI OIDC login flow.
@@ -830,21 +830,68 @@ func (h *handlerState) handleAuthCodeCallback(w http.ResponseWriter, r *http.Req
 	}()

 	var params url.Values
-	if h.useFormPost {
-		// Return HTTP 405 for anything that's not a POST.
-		if r.Method != http.MethodPost {
-			return httperr.Newf(http.StatusMethodNotAllowed, "wanted POST")
+	if h.useFormPost { // nolint:nestif
+		// Return HTTP 405 for anything that's not a POST or an OPTIONS request.
+		if r.Method != http.MethodPost && r.Method != http.MethodOptions {
+			h.logger.V(debugLogLevel).Info("Pinniped: Got unexpected request on callback listener", "method", r.Method)
+			w.WriteHeader(http.StatusMethodNotAllowed)
+			return nil // keep listening for more requests
 		}

-		// Parse and pull the response parameters from a application/x-www-form-urlencoded request body.
+		// For POST and OPTIONS requests, calculate the allowed origin for CORS.
+		issuerURL, parseErr := url.Parse(h.issuer)
+		if parseErr != nil {
+			return httperr.Wrap(http.StatusInternalServerError, "invalid issuer url", parseErr)
+		}
+		allowOrigin := issuerURL.Scheme + "://" + issuerURL.Host
+
+		if r.Method == http.MethodOptions {
+			// Google Chrome decided that it should do CORS preflight checks for this Javascript form submission POST request.
+			// See https://developer.chrome.com/blog/private-network-access-preflight/
+			origin := r.Header.Get("Origin")
+			if origin == "" {
+				// The CORS preflight request should have an origin.
+				h.logger.V(debugLogLevel).Info("Pinniped: Got OPTIONS request without origin header")
+				w.WriteHeader(http.StatusBadRequest)
+				return nil // keep listening for more requests
+			}
+			h.logger.V(debugLogLevel).Info("Pinniped: Got CORS preflight request from browser", "origin", origin)
+			// To tell the browser that it is okay to make the real POST request, return the following response.
+			w.Header().Set("Access-Control-Allow-Origin", allowOrigin)
+			w.Header().Set("Vary", "*") // supposed to use Vary when Access-Control-Allow-Origin is a specific host
+			w.Header().Set("Access-Control-Allow-Credentials", "false")
+			w.Header().Set("Access-Control-Allow-Methods", "POST, OPTIONS")
+			w.Header().Set("Access-Control-Allow-Private-Network", "true")
+			// If the browser would like to send some headers on the real request, allow them. Chrome doesn't
+			// currently send this header at the moment. This is in case some browser in the future decides to
+			// request to be allowed to send specific headers by using Access-Control-Request-Headers.
+			requestedHeaders := r.Header.Get("Access-Control-Request-Headers")
+			if requestedHeaders != "" {
+				w.Header().Set("Access-Control-Allow-Headers", requestedHeaders)
+			}
+			w.WriteHeader(http.StatusNoContent)
+			return nil // keep listening for more requests
+		} // Otherwise, this is a POST request...
+
+		// Parse and pull the response parameters from an application/x-www-form-urlencoded request body.
 		if err := r.ParseForm(); err != nil {
 			return httperr.Wrap(http.StatusBadRequest, "invalid form", err)
 		}
 		params = r.Form
+
+		// Allow CORS requests for POST so in the future our Javascript code can be updated to use the fetch API's
+		// mode "cors", and still be compatible with older CLI versions starting with those that have this code
+		// for CORS headers. Updating to use CORS would allow our Javascript code (form_post.js) to see the true
+		// http response status from this endpoint. Note that the POST response does not need to set as many CORS
+		// headers as the OPTIONS preflight response.
+		w.Header().Set("Access-Control-Allow-Origin", allowOrigin)
+		w.Header().Set("Vary", "*") // supposed to use Vary when Access-Control-Allow-Origin is a specific host
 	} else {
 		// Return HTTP 405 for anything that's not a GET.
 		if r.Method != http.MethodGet {
-			return httperr.Newf(http.StatusMethodNotAllowed, "wanted GET")
+			h.logger.V(debugLogLevel).Info("Pinniped: Got unexpected request on callback listener", "method", r.Method)
+			w.WriteHeader(http.StatusMethodNotAllowed)
+			return nil // keep listening for more requests
 		}

 		// Pull response parameters from the URL query string.
--- a/pkg/oidcclient/login_test.go
+++ b/pkg/oidcclient/login_test.go
@@ -1,4 +1,4 @@
-// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0

 package oidcclient
@@ -1697,6 +1697,8 @@ func TestHandlePasteCallback(t *testing.T) {
 	for _, tt := range tests {
 		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
 			h := &handlerState{
 				callbacks: make(chan callbackResult, 1),
 				state:     state.State("test-state"),
@@ -1738,62 +1740,161 @@ func TestHandleAuthCodeCallback(t *testing.T) {
 		}
 	}
 	tests := []struct {
-		name           string
-		method         string
-		query          string
-		body           []byte
-		contentType    string
-		opt            func(t *testing.T) Option
-		wantErr        string
-		wantHTTPStatus int
+		name    string
+		method  string
+		query   string
+		body    []byte
+		headers http.Header
+		opt     func(t *testing.T) Option
+
+		wantErr         string
+		wantHTTPStatus  int
+		wantNoCallbacks bool
+		wantHeaders     http.Header
 	}{
 		{
-			name:           "wrong method",
-			method:         "POST",
-			query:          "",
-			wantErr:        "wanted GET",
-			wantHTTPStatus: http.StatusMethodNotAllowed,
+			name:            "wrong method returns an error but keeps listening",
+			method:          http.MethodPost,
+			query:           "",
+			wantNoCallbacks: true,
+			wantHeaders:     map[string][]string{},
+			wantHTTPStatus:  http.StatusMethodNotAllowed,
 		},
 		{
-			name:           "wrong method for form_post",
-			method:         "GET",
-			query:          "",
-			opt:            withFormPostMode,
-			wantErr:        "wanted POST",
-			wantHTTPStatus: http.StatusMethodNotAllowed,
+			name:            "wrong method for form_post returns an error but keeps listening",
+			method:          http.MethodGet,
+			query:           "",
+			opt:             withFormPostMode,
+			wantNoCallbacks: true,
+			wantHeaders:     map[string][]string{},
+			wantHTTPStatus:  http.StatusMethodNotAllowed,
 		},
 		{
 			name:           "invalid form for form_post",
-			method:         "POST",
+			method:         http.MethodPost,
 			query:          "",
-			contentType:    "application/x-www-form-urlencoded",
+			headers:        map[string][]string{"Content-Type": {"application/x-www-form-urlencoded"}},
 			body:           []byte(`%`),
 			opt:            withFormPostMode,
 			wantErr:        `invalid form: invalid URL escape "%"`,
+			wantHeaders:    map[string][]string{},
 			wantHTTPStatus: http.StatusBadRequest,
 		},
 		{
 			name:           "invalid state",
 			query:          "state=invalid",
 			wantErr:        "missing or invalid state parameter",
+			wantHeaders:    map[string][]string{},
 			wantHTTPStatus: http.StatusForbidden,
 		},
 		{
 			name:           "error code from provider",
 			query:          "state=test-state&error=some_error",
 			wantErr:        `login failed with code "some_error"`,
+			wantHeaders:    map[string][]string{},
 			wantHTTPStatus: http.StatusBadRequest,
 		},
 		{
 			name:           "error code with a description from provider",
 			query:          "state=test-state&error=some_error&error_description=optional%20error%20description",
 			wantErr:        `login failed with code "some_error": optional error description`,
+			wantHeaders:    map[string][]string{},
 			wantHTTPStatus: http.StatusBadRequest,
 		},
+		{
+			name:           "in form post mode, invalid issuer url config during CORS preflight request returns an error",
+			method:         http.MethodOptions,
+			query:          "",
+			headers:        map[string][]string{"Origin": {"https://some-origin.com"}},
+			wantErr:        `invalid issuer url: parse "://bad-url": missing protocol scheme`,
+			wantHeaders:    map[string][]string{},
+			wantHTTPStatus: http.StatusInternalServerError,
+			opt: func(t *testing.T) Option {
+				return func(h *handlerState) error {
+					h.useFormPost = true
+					h.issuer = "://bad-url"
+					return nil
+				}
+			},
+		},
+		{
+			name:           "in form post mode, invalid issuer url config during POST request returns an error",
+			method:         http.MethodPost,
+			query:          "",
+			headers:        map[string][]string{"Origin": {"https://some-origin.com"}},
+			wantErr:        `invalid issuer url: parse "://bad-url": missing protocol scheme`,
+			wantHeaders:    map[string][]string{},
+			wantHTTPStatus: http.StatusInternalServerError,
+			opt: func(t *testing.T) Option {
+				return func(h *handlerState) error {
+					h.useFormPost = true
+					h.issuer = "://bad-url"
+					return nil
+				}
+			},
+		},
+		{
+			name:            "in form post mode, options request is missing origin header results in 400 and keeps listener running",
+			method:          http.MethodOptions,
+			query:           "",
+			opt:             withFormPostMode,
+			wantNoCallbacks: true,
+			wantHeaders:     map[string][]string{},
+			wantHTTPStatus:  http.StatusBadRequest,
+		},
+		{
+			name:            "in form post mode, valid CORS request responds with 402 and CORS headers and keeps listener running",
+			method:          http.MethodOptions,
+			query:           "",
+			headers:         map[string][]string{"Origin": {"https://some-origin.com"}},
+			wantNoCallbacks: true,
+			wantHTTPStatus:  http.StatusNoContent,
+			wantHeaders: map[string][]string{
+				"Access-Control-Allow-Credentials":     {"false"},
+				"Access-Control-Allow-Methods":         {"POST, OPTIONS"},
+				"Access-Control-Allow-Origin":          {"https://valid-issuer.com"},
+				"Vary":                                 {"*"},
+				"Access-Control-Allow-Private-Network": {"true"},
+			},
+			opt: func(t *testing.T) Option {
+				return func(h *handlerState) error {
+					h.useFormPost = true
+					h.issuer = "https://valid-issuer.com/with/some/path"
+					return nil
+				}
+			},
+		},
+		{
+			name:   "in form post mode, valid CORS request with Access-Control-Request-Headers responds with 402 and CORS headers including Access-Control-Allow-Headers and keeps listener running",
+			method: http.MethodOptions,
+			query:  "",
+			headers: map[string][]string{
+				"Origin":                         {"https://some-origin.com"},
+				"Access-Control-Request-Headers": {"header1, header2, header3"},
+			},
+			wantNoCallbacks: true,
+			wantHTTPStatus:  http.StatusNoContent,
+			wantHeaders: map[string][]string{
+				"Access-Control-Allow-Credentials":     {"false"},
+				"Access-Control-Allow-Methods":         {"POST, OPTIONS"},
+				"Access-Control-Allow-Origin":          {"https://valid-issuer.com"},
+				"Vary":                                 {"*"},
+				"Access-Control-Allow-Private-Network": {"true"},
+				"Access-Control-Allow-Headers":         {"header1, header2, header3"},
+			},
+			opt: func(t *testing.T) Option {
+				return func(h *handlerState) error {
+					h.useFormPost = true
+					h.issuer = "https://valid-issuer.com/with/some/path"
+					return nil
+				}
+			},
+		},
 		{
 			name:           "invalid code",
 			query:          "state=test-state&code=invalid",
 			wantErr:        "could not complete code exchange: some exchange error",
+			wantHeaders:    map[string][]string{},
 			wantHTTPStatus: http.StatusBadRequest,
 			opt: func(t *testing.T) Option {
 				return func(h *handlerState) error {
@@ -1810,8 +1911,10 @@ func TestHandleAuthCodeCallback(t *testing.T) {
 			},
 		},
 		{
-			name:  "valid",
-			query: "state=test-state&code=valid",
+			name:           "valid",
+			query:          "state=test-state&code=valid",
+			wantHTTPStatus: http.StatusOK,
+			wantHeaders:    map[string][]string{"Content-Type": {"text/plain; charset=utf-8"}},
 			opt: func(t *testing.T) Option {
 				return func(h *handlerState) error {
 					h.oauth2Config = &oauth2.Config{RedirectURL: testRedirectURI}
@@ -1827,10 +1930,45 @@ func TestHandleAuthCodeCallback(t *testing.T) {
 			},
 		},
 		{
-			name:        "valid form_post",
-			method:      http.MethodPost,
-			contentType: "application/x-www-form-urlencoded",
-			body:        []byte(`state=test-state&code=valid`),
+			name:    "valid form_post",
+			method:  http.MethodPost,
+			headers: map[string][]string{"Content-Type": {"application/x-www-form-urlencoded"}},
+			body:    []byte(`state=test-state&code=valid`),
+			wantHeaders: map[string][]string{
+				"Access-Control-Allow-Origin": {"https://valid-issuer.com"},
+				"Vary":                        {"*"},
+				"Content-Type":                {"text/plain; charset=utf-8"},
+			},
+			wantHTTPStatus: http.StatusOK,
+			opt: func(t *testing.T) Option {
+				return func(h *handlerState) error {
+					h.useFormPost = true
+					h.oauth2Config = &oauth2.Config{RedirectURL: testRedirectURI}
+					h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) provider.UpstreamOIDCIdentityProviderI {
+						mock := mockUpstream(t)
+						mock.EXPECT().
+							ExchangeAuthcodeAndValidateTokens(gomock.Any(), "valid", pkce.Code("test-pkce"), nonce.Nonce("test-nonce"), testRedirectURI).
+							Return(&oidctypes.Token{IDToken: &oidctypes.IDToken{Token: "test-id-token"}}, nil)
+						return mock
+					}
+					return nil
+				}
+			},
+		},
+		{
+			name:   "valid form_post made with the same origin headers that would be used by a Javascript fetch client using mode=cors",
+			method: http.MethodPost,
+			headers: map[string][]string{
+				"Content-Type": {"application/x-www-form-urlencoded"},
+				"Origin":       {"https://some-origin.com"},
+			},
+			body: []byte(`state=test-state&code=valid`),
+			wantHeaders: map[string][]string{
+				"Access-Control-Allow-Origin": {"https://valid-issuer.com"},
+				"Vary":                        {"*"},
+				"Content-Type":                {"text/plain; charset=utf-8"},
+			},
+			wantHTTPStatus: http.StatusOK,
 			opt: func(t *testing.T) Option {
 				return func(h *handlerState) error {
 					h.useFormPost = true
@@ -1850,11 +1988,15 @@ func TestHandleAuthCodeCallback(t *testing.T) {
 	for _, tt := range tests {
 		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
 			h := &handlerState{
 				callbacks: make(chan callbackResult, 1),
 				state:     state.State("test-state"),
 				pkce:      pkce.Code("test-pkce"),
 				nonce:     nonce.Nonce("test-nonce"),
+				logger:    testlogger.New(t).Logger,
+				issuer:    "https://valid-issuer.com/with/some/path",
 			}
 			if tt.opt != nil {
 				require.NoError(t, tt.opt(t)(h))
@@ -1870,8 +2012,8 @@ func TestHandleAuthCodeCallback(t *testing.T) {
 			if tt.method != "" {
 				req.Method = tt.method
 			}
-			if tt.contentType != "" {
-				req.Header.Set("Content-Type", tt.contentType)
+			if tt.headers != nil {
+				req.Header = tt.headers
 			}

 			err = h.handleAuthCodeCallback(resp, req)
@@ -1884,11 +2026,19 @@ func TestHandleAuthCodeCallback(t *testing.T) {
 				}
 			} else {
 				require.NoError(t, err)
+				require.Equal(t, tt.wantHTTPStatus, resp.Code)
 			}

+			if tt.wantHeaders != nil {
+				require.Equal(t, tt.wantHeaders, resp.Header())
+			}
+
+			gotCallback := false
 			select {
 			case <-time.After(1 * time.Second):
-				require.Fail(t, "timed out waiting to receive from callbacks channel")
+				if !tt.wantNoCallbacks {
+					require.Fail(t, "timed out waiting to receive from callbacks channel")
+				}
 			case result := <-h.callbacks:
 				if tt.wantErr != "" {
 					require.EqualError(t, result.err, tt.wantErr)
@@ -1897,7 +2047,9 @@ func TestHandleAuthCodeCallback(t *testing.T) {
 				require.NoError(t, result.err)
 				require.NotNil(t, result.token)
 				require.Equal(t, result.token.IDToken.Token, "test-id-token")
+				gotCallback = true
 			}
+			require.Equal(t, tt.wantNoCallbacks, !gotCallback)
 		})
 	}
 }
--- a/test/integration/cli_test.go
+++ b/test/integration/cli_test.go
@@ -1,4 +1,4 @@
-// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 package integration

@@ -7,7 +7,6 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -286,32 +285,21 @@ func runPinnipedLoginOIDC(
 	t.Logf("starting CLI subprocess")
 	require.NoError(t, cmd.Start())
 	t.Cleanup(func() {
-		err := cmd.Wait()
+		err := cmd.Wait() // handles closing of file descriptors
 		t.Logf("CLI subprocess exited with code %d", cmd.ProcessState.ExitCode())
 		require.NoErrorf(t, err, "CLI process did not exit cleanly")
 	})

 	// Start a background goroutine to read stderr from the CLI and parse out the login URL.
-	loginURLChan := make(chan string)
-	spawnTestGoroutine(t, func() (err error) {
-		t.Helper()
-		defer func() {
-			closeErr := stderr.Close()
-			if closeErr == nil || errors.Is(closeErr, os.ErrClosed) {
-				return
-			}
-			if err == nil {
-				err = fmt.Errorf("stderr stream closed with error: %w", closeErr)
-			}
-		}()
-
+	loginURLChan := make(chan string, 1)
+	spawnTestGoroutine(ctx, t, func() error {
 		reader := bufio.NewReader(testlib.NewLoggerReader(t, "stderr", stderr))

 		scanner := bufio.NewScanner(reader)
 		for scanner.Scan() {
 			loginURL, err := url.Parse(strings.TrimSpace(scanner.Text()))
 			if err == nil && loginURL.Scheme == "https" {
-				loginURLChan <- loginURL.String()
+				loginURLChan <- loginURL.String() // this channel is buffered so this will not block
 				return nil
 			}
 		}
@@ -320,23 +308,14 @@ func runPinnipedLoginOIDC(
 	})

 	// Start a background goroutine to read stdout from the CLI and parse out an ExecCredential.
-	credOutputChan := make(chan clientauthenticationv1beta1.ExecCredential)
-	spawnTestGoroutine(t, func() (err error) {
-		defer func() {
-			closeErr := stdout.Close()
-			if closeErr == nil || errors.Is(closeErr, os.ErrClosed) {
-				return
-			}
-			if err == nil {
-				err = fmt.Errorf("stdout stream closed with error: %w", closeErr)
-			}
-		}()
+	credOutputChan := make(chan clientauthenticationv1beta1.ExecCredential, 1)
+	spawnTestGoroutine(ctx, t, func() error {
 		reader := bufio.NewReader(testlib.NewLoggerReader(t, "stdout", stdout))
 		var out clientauthenticationv1beta1.ExecCredential
 		if err := json.NewDecoder(reader).Decode(&out); err != nil {
 			return fmt.Errorf("could not read ExecCredential from stdout: %w", err)
 		}
-		credOutputChan <- out
+		credOutputChan <- out // this channel is buffered so this will not block
 		return readAndExpectEmpty(reader)
 	})

@@ -391,11 +370,33 @@ func readAndExpectEmpty(r io.Reader) (err error) {
 	return nil
 }

-func spawnTestGoroutine(t *testing.T, f func() error) {
+// Note: Callers should ensure that f eventually returns, otherwise this helper will leak a go routine.
+func spawnTestGoroutine(ctx context.Context, t *testing.T, f func() error) {
 	t.Helper()
+
 	var eg errgroup.Group
 	t.Cleanup(func() {
-		require.NoError(t, eg.Wait(), "background goroutine failed")
+		egCh := make(chan error, 1) // do not block the go routine from exiting even after the select has completed
+		go func() {
+			egCh <- eg.Wait()
+		}()
+
+		leewayCh := make(chan struct{})
+		go func() {
+			<-ctx.Done()
+			// give f up to 30 seconds after the context is canceled to return
+			// this prevents "race" conditions where f is orchestrated via the same context
+			time.Sleep(30 * time.Second)
+			close(leewayCh)
+		}()
+
+		select {
+		case <-leewayCh:
+			t.Errorf("background goroutine hung: %v", ctx.Err())
+
+		case err := <-egCh:
+			require.NoError(t, err, "background goroutine failed")
+		}
 	})
 	eg.Go(f)
 }
--- a/test/integration/concierge_impersonation_proxy_test.go
+++ b/test/integration/concierge_impersonation_proxy_test.go
@@ -1,4 +1,4 @@
-// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0

 package integration
@@ -938,6 +938,9 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl
 			namespaceName := testlib.CreateNamespace(ctx, t, "impersonation").Name
 			kubeClient := adminClient.CoreV1()
 			saName, _, saUID := createServiceAccountToken(ctx, t, adminClient, namespaceName)
+			expectedUsername := serviceaccount.MakeUsername(namespaceName, saName)
+			expectedUID := string(saUID)
+			expectedGroups := []string{"system:serviceaccounts", "system:serviceaccounts:" + namespaceName, "system:authenticated"}

 			_, tokenRequestProbeErr := kubeClient.ServiceAccounts(namespaceName).CreateToken(ctx, saName, &authenticationv1.TokenRequest{}, metav1.CreateOptions{})
 			if k8serrors.IsNotFound(tokenRequestProbeErr) && tokenRequestProbeErr.Error() == "the server could not find the requested resource" {
@@ -1002,8 +1005,8 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl
 			// new service account tokens include the pod info in the extra fields
 			require.Equal(t,
 				expectedWhoAmIRequestResponse(
-					serviceaccount.MakeUsername(namespaceName, saName),
-					[]string{"system:serviceaccounts", "system:serviceaccounts:" + namespaceName, "system:authenticated"},
+					expectedUsername,
+					expectedGroups,
 					map[string]identityv1alpha1.ExtraValue{
 						"authentication.kubernetes.io/pod-name": {pod.Name},
 						"authentication.kubernetes.io/pod-uid":  {string(pod.UID)},
@@ -1017,7 +1020,7 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl
 				rbacv1.Subject{Kind: rbacv1.ServiceAccountKind, Name: saName, Namespace: namespaceName},
 				rbacv1.RoleRef{Kind: "ClusterRole", APIGroup: rbacv1.GroupName, Name: "system:node-bootstrapper"},
 			)
-			testlib.WaitForUserToHaveAccess(t, serviceaccount.MakeUsername(namespaceName, saName), []string{}, &authorizationv1.ResourceAttributes{
+			testlib.WaitForUserToHaveAccess(t, expectedUsername, []string{}, &authorizationv1.ResourceAttributes{
 				Verb: "create", Group: certificatesv1.GroupName, Version: "*", Resource: "certificatesigningrequests",
 			})

@@ -1041,20 +1044,34 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl
 			)
 			require.NoError(t, err)

-			saCSR, err := impersonationProxySAClient.Kubernetes.CertificatesV1beta1().CertificateSigningRequests().Get(ctx, csrName, metav1.GetOptions{})
-			require.NoError(t, err)
-
-			err = adminClient.CertificatesV1beta1().CertificateSigningRequests().Delete(ctx, csrName, metav1.DeleteOptions{})
-			require.NoError(t, err)
-
-			// make sure the user info that the CSR captured matches the SA, including the UID
-			require.Equal(t, serviceaccount.MakeUsername(namespaceName, saName), saCSR.Spec.Username)
-			require.Equal(t, string(saUID), saCSR.Spec.UID)
-			require.Equal(t, []string{"system:serviceaccounts", "system:serviceaccounts:" + namespaceName, "system:authenticated"}, saCSR.Spec.Groups)
-			require.Equal(t, map[string]certificatesv1beta1.ExtraValue{
-				"authentication.kubernetes.io/pod-name": {pod.Name},
-				"authentication.kubernetes.io/pod-uid":  {string(pod.UID)},
-			}, saCSR.Spec.Extra)
+			if testutil.KubeServerSupportsCertificatesV1API(t, adminClient.Discovery()) {
+				saCSR, err := impersonationProxySAClient.Kubernetes.CertificatesV1().CertificateSigningRequests().Get(ctx, csrName, metav1.GetOptions{})
+				require.NoError(t, err)
+				err = adminClient.CertificatesV1().CertificateSigningRequests().Delete(ctx, csrName, metav1.DeleteOptions{})
+				require.NoError(t, err)
+				// make sure the user info that the CSR captured matches the SA, including the UID
+				require.Equal(t, expectedUsername, saCSR.Spec.Username)
+				require.Equal(t, expectedUID, saCSR.Spec.UID)
+				require.Equal(t, expectedGroups, saCSR.Spec.Groups)
+				require.Equal(t, map[string]certificatesv1.ExtraValue{
+					"authentication.kubernetes.io/pod-name": {pod.Name},
+					"authentication.kubernetes.io/pod-uid":  {string(pod.UID)},
+				}, saCSR.Spec.Extra)
+			} else {
+				// On old Kubernetes clusters use CertificatesV1beta1
+				saCSR, err := impersonationProxySAClient.Kubernetes.CertificatesV1beta1().CertificateSigningRequests().Get(ctx, csrName, metav1.GetOptions{})
+				require.NoError(t, err)
+				err = adminClient.CertificatesV1beta1().CertificateSigningRequests().Delete(ctx, csrName, metav1.DeleteOptions{})
+				require.NoError(t, err)
+				// make sure the user info that the CSR captured matches the SA, including the UID
+				require.Equal(t, expectedUsername, saCSR.Spec.Username)
+				require.Equal(t, expectedUID, saCSR.Spec.UID)
+				require.Equal(t, expectedGroups, saCSR.Spec.Groups)
+				require.Equal(t, map[string]certificatesv1beta1.ExtraValue{
+					"authentication.kubernetes.io/pod-name": {pod.Name},
+					"authentication.kubernetes.io/pod-uid":  {string(pod.UID)},
+				}, saCSR.Spec.Extra)
+			}
 		})

 		t.Run("kubectl as a client", func(t *testing.T) {
@@ -2416,7 +2433,7 @@ func getCredForConfig(t *testing.T, config *rest.Config) *loginv1alpha1.ClusterC
 	return out
 }

-func getUIDAndExtraViaCSR(ctx context.Context, t *testing.T, uid string, client kubernetes.Interface) (string, map[string]certificatesv1beta1.ExtraValue) {
+func getUIDAndExtraViaCSR(ctx context.Context, t *testing.T, uid string, client kubernetes.Interface) (string, map[string][]string) {
 	t.Helper()

 	privateKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
@@ -2439,18 +2456,43 @@ func getUIDAndExtraViaCSR(ctx context.Context, t *testing.T, uid string, client
 	)
 	require.NoError(t, err)

-	csReq, err := client.CertificatesV1beta1().CertificateSigningRequests().Get(ctx, csrName, metav1.GetOptions{})
-	require.NoError(t, err)
-
-	err = client.CertificatesV1beta1().CertificateSigningRequests().Delete(ctx, csrName, metav1.DeleteOptions{})
-	require.NoError(t, err)
-
 	outUID := uid // in the future this may not be empty on some clusters
-	if len(outUID) == 0 {
-		outUID = csReq.Spec.UID
+	extrasAsStrings := map[string][]string{}
+
+	if testutil.KubeServerSupportsCertificatesV1API(t, client.Discovery()) {
+		csReq, err := client.CertificatesV1().CertificateSigningRequests().Get(ctx, csrName, metav1.GetOptions{})
+		require.NoError(t, err)
+
+		err = client.CertificatesV1().CertificateSigningRequests().Delete(ctx, csrName, metav1.DeleteOptions{})
+		require.NoError(t, err)
+
+		if len(outUID) == 0 {
+			outUID = csReq.Spec.UID
+		}
+
+		// Convert each `ExtraValue` to `[]string` to return, so we don't have to deal with v1beta1 types versus v1 types
+		for k, v := range csReq.Spec.Extra {
+			extrasAsStrings[k] = v
+		}
+	} else {
+		// On old Kubernetes clusters use CertificatesV1beta1
+		csReq, err := client.CertificatesV1beta1().CertificateSigningRequests().Get(ctx, csrName, metav1.GetOptions{})
+		require.NoError(t, err)
+
+		err = client.CertificatesV1beta1().CertificateSigningRequests().Delete(ctx, csrName, metav1.DeleteOptions{})
+		require.NoError(t, err)
+
+		if len(outUID) == 0 {
+			outUID = csReq.Spec.UID
+		}
+
+		// Convert each `ExtraValue` to `[]string` to return, so we don't have to deal with v1beta1 types versus v1 types
+		for k, v := range csReq.Spec.Extra {
+			extrasAsStrings[k] = v
+		}
 	}

-	return outUID, csReq.Spec.Extra
+	return outUID, extrasAsStrings
 }

 func parallelIfNotEKS(t *testing.T) {
--- a/test/integration/e2e_test.go
+++ b/test/integration/e2e_test.go
@@ -1,4 +1,4 @@
-// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 package integration

@@ -19,6 +19,7 @@ import (
 	"regexp"
 	"sort"
 	"strings"
+	"sync/atomic"
 	"testing"
 	"time"

@@ -31,6 +32,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	utilerrors "k8s.io/apimachinery/pkg/util/errors"

 	authv1alpha "go.pinniped.dev/generated/latest/apis/concierge/authentication/v1alpha1"
 	configv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/config/v1alpha1"
@@ -47,10 +49,10 @@ import (
 )

 // TestE2EFullIntegration tests a full integration scenario that combines the supervisor, concierge, and CLI.
-func TestE2EFullIntegration(t *testing.T) { // nolint:gocyclo
+func TestE2EFullIntegration(t *testing.T) {
 	env := testlib.IntegrationEnv(t)

-	ctx, cancelFunc := context.WithTimeout(context.Background(), 15*time.Minute)
+	ctx, cancelFunc := context.WithTimeout(context.Background(), 10*time.Minute)
 	defer cancelFunc()

 	// Build pinniped CLI.
@@ -107,6 +109,9 @@ func TestE2EFullIntegration(t *testing.T) { // nolint:gocyclo

 	// Add an OIDC upstream IDP and try using it to authenticate during kubectl commands.
 	t.Run("with Supervisor OIDC upstream IDP and automatic flow", func(t *testing.T) {
+		testCtx, cancel := context.WithTimeout(ctx, 2*time.Minute)
+		t.Cleanup(cancel)
+
 		// Start a fresh browser driver because we don't want to share cookies between the various tests in this file.
 		page := browsertest.Open(t)

@@ -158,48 +163,58 @@ func TestE2EFullIntegration(t *testing.T) { // nolint:gocyclo

 		// Run "kubectl get namespaces" which should trigger a browser login via the plugin.
 		start := time.Now()
-		kubectlCmd := exec.CommandContext(ctx, "kubectl", "get", "namespace", "--kubeconfig", kubeconfigPath)
+		kubectlCmd := exec.CommandContext(testCtx, "kubectl", "get", "namespace", "--kubeconfig", kubeconfigPath, "-v", "6")
 		kubectlCmd.Env = append(os.Environ(), env.ProxyEnv()...)
-		stderrPipe, err := kubectlCmd.StderrPipe()
+
+		// Wrap the stdout and stderr pipes with TeeReaders which will copy each incremental read to an
+		// in-memory buffer, so we can have the full output available to us at the end.
+		originalStderrPipe, err := kubectlCmd.StderrPipe()
 		require.NoError(t, err)
-		stdoutPipe, err := kubectlCmd.StdoutPipe()
+		originalStdoutPipe, err := kubectlCmd.StdoutPipe()
 		require.NoError(t, err)
+		var stderrPipeBuf, stdoutPipeBuf bytes.Buffer
+		stderrPipe := io.TeeReader(originalStderrPipe, &stderrPipeBuf)
+		stdoutPipe := io.TeeReader(originalStdoutPipe, &stdoutPipeBuf)

 		t.Logf("starting kubectl subprocess")
 		require.NoError(t, kubectlCmd.Start())
 		t.Cleanup(func() {
-			err := kubectlCmd.Wait()
+			// Consume readers so that the tee buffers will contain all the output so far.
+			_, stdoutReadAllErr := readAllCtx(testCtx, stdoutPipe)
+			_, stderrReadAllErr := readAllCtx(testCtx, stderrPipe)
+
+			// Note that Wait closes the stdout/stderr pipes, so we don't need to close them ourselves.
+			waitErr := kubectlCmd.Wait()
 			t.Logf("kubectl subprocess exited with code %d", kubectlCmd.ProcessState.ExitCode())
-			stdout, stdoutErr := ioutil.ReadAll(stdoutPipe)
-			if stdoutErr != nil {
-				stdout = []byte("<error reading stdout: " + stdoutErr.Error() + ">")
+
+			// Upon failure, print the full output so far of the kubectl command.
+			var testAlreadyFailedErr error
+			if t.Failed() {
+				testAlreadyFailedErr = errors.New("test failed prior to clean up function")
 			}
-			stderr, stderrErr := ioutil.ReadAll(stderrPipe)
-			if stderrErr != nil {
-				stderr = []byte("<error reading stderr: " + stderrErr.Error() + ">")
+			cleanupErrs := utilerrors.NewAggregate([]error{waitErr, stdoutReadAllErr, stderrReadAllErr, testAlreadyFailedErr})
+
+			if cleanupErrs != nil {
+				t.Logf("kubectl stdout was:\n----start of stdout\n%s\n----end of stdout", stdoutPipeBuf.String())
+				t.Logf("kubectl stderr was:\n----start of stderr\n%s\n----end of stderr", stderrPipeBuf.String())
 			}
-			require.NoErrorf(t, err, "kubectl process did not exit cleanly, stdout/stderr: %q/%q", string(stdout), string(stderr))
+			require.NoErrorf(t, cleanupErrs, "kubectl process did not exit cleanly and/or the test failed. "+
+				"Note: if kubectl's first call to the Pinniped CLI results in the Pinniped CLI returning an error, "+
+				"then kubectl may call the Pinniped CLI again, which may hang because it will wait for the user "+
+				"to finish the login. This test will kill the kubectl process after a timeout. In this case, the "+
+				" kubectl output printed above will include multiple prompts for the user to enter their authcode.",
+			)
 		})

 		// Start a background goroutine to read stderr from the CLI and parse out the login URL.
-		loginURLChan := make(chan string)
-		spawnTestGoroutine(t, func() (err error) {
-			defer func() {
-				closeErr := stderrPipe.Close()
-				if closeErr == nil || errors.Is(closeErr, os.ErrClosed) {
-					return
-				}
-				if err == nil {
-					err = fmt.Errorf("stderr stream closed with error: %w", closeErr)
-				}
-			}()
-
+		loginURLChan := make(chan string, 1)
+		spawnTestGoroutine(testCtx, t, func() error {
 			reader := bufio.NewReader(testlib.NewLoggerReader(t, "stderr", stderrPipe))
 			scanner := bufio.NewScanner(reader)
 			for scanner.Scan() {
 				loginURL, err := url.Parse(strings.TrimSpace(scanner.Text()))
 				if err == nil && loginURL.Scheme == "https" {
-					loginURLChan <- loginURL.String()
+					loginURLChan <- loginURL.String() // this channel is buffered so this will not block
 					return nil
 				}
 			}
@@ -207,23 +222,14 @@ func TestE2EFullIntegration(t *testing.T) { // nolint:gocyclo
 		})

 		// Start a background goroutine to read stdout from kubectl and return the result as a string.
-		kubectlOutputChan := make(chan string)
-		spawnTestGoroutine(t, func() (err error) {
-			defer func() {
-				closeErr := stdoutPipe.Close()
-				if closeErr == nil || errors.Is(closeErr, os.ErrClosed) {
-					return
-				}
-				if err == nil {
-					err = fmt.Errorf("stdout stream closed with error: %w", closeErr)
-				}
-			}()
-			output, err := ioutil.ReadAll(stdoutPipe)
+		kubectlOutputChan := make(chan string, 1)
+		spawnTestGoroutine(testCtx, t, func() error {
+			output, err := readAllCtx(testCtx, stdoutPipe)
 			if err != nil {
 				return err
 			}
 			t.Logf("kubectl output:\n%s\n", output)
-			kubectlOutputChan <- string(output)
+			kubectlOutputChan <- string(output) // this channel is buffered so this will not block
 			return nil
 		})

@@ -235,7 +241,7 @@ func TestE2EFullIntegration(t *testing.T) { // nolint:gocyclo
 			require.Fail(t, "timed out waiting for login URL")
 		case loginURL = <-loginURLChan:
 		}
-		t.Logf("navigating to login page")
+		t.Logf("navigating to login page: %q", loginURL)
 		require.NoError(t, page.Navigate(loginURL))

 		// Expect to be redirected to the upstream provider and log in.
@@ -261,7 +267,7 @@ func TestE2EFullIntegration(t *testing.T) { // nolint:gocyclo

 		t.Logf("first kubectl command took %s", time.Since(start).String())

-		requireUserCanUseKubectlWithoutAuthenticatingAgain(ctx, t, env,
+		requireUserCanUseKubectlWithoutAuthenticatingAgain(testCtx, t, env,
 			downstream,
 			kubeconfigPath,
 			sessionCachePath,
@@ -1056,3 +1062,42 @@ func getSecretNameFromSignature(t *testing.T, signature string, typeLabel string
 	signatureAsValidName := strings.ToLower(b32.EncodeToString(signatureBytes))
 	return fmt.Sprintf("pinniped-storage-%s-%s", typeLabel, signatureAsValidName)
 }
+
+func readAllCtx(ctx context.Context, r io.Reader) ([]byte, error) {
+	errCh := make(chan error, 1)
+	data := &atomic.Value{}
+	go func() { // copied from io.ReadAll and modified to use the atomic.Value above
+		b := make([]byte, 0, 512)
+		data.Store(string(b)) // cast to string to make a copy of the byte slice
+		for {
+			if len(b) == cap(b) {
+				// Add more capacity (let append pick how much).
+				b = append(b, 0)[:len(b)]
+				data.Store(string(b)) // cast to string to make a copy of the byte slice
+			}
+			n, err := r.Read(b[len(b):cap(b)])
+			b = b[:len(b)+n]
+			data.Store(string(b)) // cast to string to make a copy of the byte slice
+			if err != nil {
+				if err == io.EOF {
+					err = nil
+				}
+				errCh <- err
+				return
+			}
+		}
+	}()
+
+	select {
+	case <-ctx.Done():
+		b, _ := data.Load().(string)
+		return nil, fmt.Errorf("failed to complete read all: %w, data read so far:\n%q", ctx.Err(), b)
+
+	case err := <-errCh:
+		b, _ := data.Load().(string)
+		if len(b) == 0 {
+			return nil, err
+		}
+		return []byte(b), err
+	}
+}
--- a/test/integration/formposthtml_test.go
+++ b/test/integration/formposthtml_test.go
@@ -1,4 +1,4 @@
-// Copyright 2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0

 package integration
@@ -60,6 +60,10 @@ func TestFormPostHTML_Parallel(t *testing.T) {
 		//
 		// This case is fairly unlikely in practice, and if the CLI encounters
 		// an error it can also expose it via stderr anyway.
+		//
+		// In the future, we could change the Javascript code to use mode 'cors'
+		// because we have upgraded our CLI callback endpoint to handle CORS,
+		// and then we could change this to formpostExpectManualState().
 		formpostExpectSuccessState(t, page)
 	})

@@ -108,6 +112,19 @@ func formpostCallbackServer(t *testing.T) (string, func(*testing.T, url.Values))
 	results := make(chan url.Values)

 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// 404 for any other requests aside from POSTs. We do not need to support CORS preflight OPTIONS
+		// requests for this test because both the web page and the callback are on 127.0.0.1 (same origin).
+		if r.Method != http.MethodPost {
+			t.Logf("test callback server got unexpeted request method")
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+
+		// Allow CORS requests. This will be needed for this test in the future if we change
+		// the Javascript code from using mode 'no-cors' to instead use mode 'cors'. At the
+		// moment it should be ignored by the browser.
+		w.Header().Set("Access-Control-Allow-Origin", "*")
+
 		assert.NoError(t, r.ParseForm())

 		// Extract only the POST parameters (r.Form also contains URL query parameters).
--- a/test/integration/whoami_test.go
+++ b/test/integration/whoami_test.go
@@ -1,4 +1,4 @@
-// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 package integration

@@ -26,6 +26,7 @@ import (
 	"k8s.io/client-go/util/keyutil"

 	identityv1alpha1 "go.pinniped.dev/generated/latest/apis/concierge/identity/v1alpha1"
+	"go.pinniped.dev/internal/testutil"
 	"go.pinniped.dev/test/testlib"
 )

@@ -281,28 +282,53 @@ func TestWhoAmI_CSR_Parallel(t *testing.T) {
 	)
 	require.NoError(t, err)

+	useCertificatesV1API := testutil.KubeServerSupportsCertificatesV1API(t, kubeClient.Discovery())
+
 	t.Cleanup(func() {
-		require.NoError(t, kubeClient.CertificatesV1beta1().CertificateSigningRequests().
-			Delete(context.Background(), csrName, metav1.DeleteOptions{}))
+		if useCertificatesV1API {
+			require.NoError(t, kubeClient.CertificatesV1().CertificateSigningRequests().
+				Delete(context.Background(), csrName, metav1.DeleteOptions{}))
+		} else {
+			// On old clusters use v1beta1
+			require.NoError(t, kubeClient.CertificatesV1beta1().CertificateSigningRequests().
+				Delete(context.Background(), csrName, metav1.DeleteOptions{}))
+		}
 	})

-	// this is a blind update with no resource version checks, which is only safe during tests
-	// use the beta CSR API to support older clusters
-	_, err = kubeClient.CertificatesV1beta1().CertificateSigningRequests().UpdateApproval(ctx, &certificatesv1beta1.CertificateSigningRequest{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: csrName,
-		},
-		Status: certificatesv1beta1.CertificateSigningRequestStatus{
-			Conditions: []certificatesv1beta1.CertificateSigningRequestCondition{
-				{
-					Type:   certificatesv1beta1.CertificateApproved,
-					Status: corev1.ConditionTrue,
-					Reason: "WhoAmICSRTest",
+	if useCertificatesV1API {
+		_, err = kubeClient.CertificatesV1().CertificateSigningRequests().UpdateApproval(ctx, csrName, &certificatesv1.CertificateSigningRequest{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: csrName,
+			},
+			Status: certificatesv1.CertificateSigningRequestStatus{
+				Conditions: []certificatesv1.CertificateSigningRequestCondition{
+					{
+						Type:   certificatesv1.CertificateApproved,
+						Status: corev1.ConditionTrue,
+						Reason: "WhoAmICSRTest",
+					},
 				},
 			},
-		},
-	}, metav1.UpdateOptions{})
-	require.NoError(t, err)
+		}, metav1.UpdateOptions{})
+		require.NoError(t, err)
+	} else {
+		// On old Kubernetes clusters use CertificatesV1beta1
+		_, err = kubeClient.CertificatesV1beta1().CertificateSigningRequests().UpdateApproval(ctx, &certificatesv1beta1.CertificateSigningRequest{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: csrName,
+			},
+			Status: certificatesv1beta1.CertificateSigningRequestStatus{
+				Conditions: []certificatesv1beta1.CertificateSigningRequestCondition{
+					{
+						Type:   certificatesv1beta1.CertificateApproved,
+						Status: corev1.ConditionTrue,
+						Reason: "WhoAmICSRTest",
+					},
+				},
+			},
+		}, metav1.UpdateOptions{})
+		require.NoError(t, err)
+	}

 	crtPEM, err := csr.WaitForCertificate(ctx, kubeClient, csrName, csrUID)
 	require.NoError(t, err)
--- a/test/testlib/browsertest/browsertest.go
+++ b/test/testlib/browsertest/browsertest.go
@@ -1,4 +1,4 @@
-// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0

 // Package browsertest provides integration test helpers for our browser-based tests.
@@ -119,7 +119,7 @@ func LoginToUpstream(t *testing.T, page *agouti.Page, upstream testlib.TestOIDCU
 		{
 			Name:                "Okta",
 			IssuerPattern:       regexp.MustCompile(`\Ahttps://.+\.okta\.com/.+\z`),
-			LoginPagePattern:    regexp.MustCompile(`\Ahttps://.+\.okta\.com/.+\z`),
+			LoginPagePattern:    regexp.MustCompile(`\Ahttps://.+\.okta\.com/.*\z`),
 			UsernameSelector:    "input#okta-signin-username",
 			PasswordSelector:    "input#okta-signin-password",
 			LoginButtonSelector: "input#okta-signin-submit",
--- a/test/testlib/env.go
+++ b/test/testlib/env.go
@@ -1,4 +1,4 @@
-// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0

 package testlib
@@ -247,7 +247,7 @@ func loadEnvVars(t *testing.T, result *TestEnv) {
 	result.SupervisorUpstreamOIDC = TestOIDCUpstream{
 		Issuer:           needEnv(t, "PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_ISSUER"),
 		CABundle:         base64Decoded(t, os.Getenv("PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_ISSUER_CA_BUNDLE")),
-		AdditionalScopes: strings.Fields(os.Getenv("PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_ADDITIONAL_SCOPES")),
+		AdditionalScopes: filterEmpty(strings.Split(strings.ReplaceAll(os.Getenv("PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_ADDITIONAL_SCOPES"), " ", ""), ",")),
 		UsernameClaim:    os.Getenv("PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_USERNAME_CLAIM"),
 		GroupsClaim:      os.Getenv("PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_GROUPS_CLAIM"),
 		ClientID:         needEnv(t, "PINNIPED_TEST_SUPERVISOR_UPSTREAM_OIDC_CLIENT_ID"),
Author	SHA1	Message	Date
Margo Crawford	4d6a2af894	Make linter happy by deleting unused gocyclo	2022-02-09 10:38:28 -08:00
Ryan Richard	dab653f8df	Fix form_post.js mistake from recent commit; Better CORS on callback (cherry picked from commit `5d79d4b9dc`)	2022-02-09 10:26:24 -08:00
Ryan Richard	8698d71809	Use "-v6" for kubectl for an e2e test so we can get more failure output (cherry picked from commit `cd825c5e51`)	2022-02-09 10:21:23 -08:00
Monis Khan	96d4d3ec7c	e2e_test: handle hung go routines and readers Signed-off-by: Monis Khan <mok@vmware.com> (cherry picked from commit `8ee461ae8a`)	2022-02-09 10:21:23 -08:00
Mo Khan	7c87d7447c	TestE2EFullIntegration: reduce timeout This causes the test to timeout before concourse terminates the entire test run. (cherry picked from commit `1388183bf1`)	2022-02-09 10:21:23 -08:00
Ryan Richard	a2e578bdbb	Fix JS bug: form post UI shows manual copy/paste UI upon failed callback When the POST to the CLI's localhost callback endpoint results in a non-2XX status code, then treat that as a failed login attempt and automatically show the manual copy/paste UI. (cherry picked from commit `6781bfd7d8`)	2022-02-09 10:21:18 -08:00
Ryan Richard	366782ab75	Capture and print the full kubectl output in an e2e test upon failure (cherry picked from commit `aa56f174db`)	2022-02-09 10:10:21 -08:00
Ryan Richard	e4e764860a	Keep the CLI localhost listener running after requests with wrong verb Just in case some future browser change sends some new kind of request to our CLI, just ignore them by returning StatusMethodNotAllowed and continuing to listen. (cherry picked from commit `3c7e387137`)	2022-02-09 10:10:21 -08:00
Ryan Richard	bb71545dee	Fix a bug in the e2e tests When the test was going to fail, a goroutine would accidentally block on writing to an unbuffered channel, and the spawnTestGoroutine helper would wait for that goroutine to end on cleanup, causing the test to hang forever while it was trying to fail. (cherry picked from commit `2b93fdf357`)	2022-02-09 10:10:21 -08:00
Ryan Richard	19ec85c84e	Add CORS request handling to CLI's localhost listener This is to support the new changes in Google Chrome v98 which now performs CORS preflight requests for the Javascript form submission on the Supervisor's login page, even though the form is being submitted to a localhost listener. (cherry picked from commit `7b97f1533e`)	2022-02-09 10:10:21 -08:00
Margo Crawford	427eef2038	impersonation proxy test needs new kube_server_compatibility stuff	2022-02-08 14:37:14 -08:00
Margo Crawford	28169637c8	Update whoami test to use kube_server_compatibility stuff	2022-02-08 14:10:45 -08:00
Margo Crawford	a5b83c90a6	Fix additional scopes parsing for integration tests	2022-02-08 10:52:38 -08:00