From e4e764860af673e42d4d4326a2eb7a84310a75f4 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Mon, 7 Feb 2022 13:32:31 -0800 Subject: [PATCH] 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 3c7e387137a880b3cffed05083e467ec6f4f6719) --- pkg/oidcclient/login.go | 10 +++++++--- pkg/oidcclient/login_test.go | 25 ++++++++++++++----------- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/pkg/oidcclient/login.go b/pkg/oidcclient/login.go index 4521000b1..53334af2f 100644 --- a/pkg/oidcclient/login.go +++ b/pkg/oidcclient/login.go @@ -864,10 +864,12 @@ func (h *handlerState) handleAuthCodeCallback(w http.ResponseWriter, r *http.Req // Return HTTP 405 for anything that's not a POST. if r.Method != http.MethodPost { - return httperr.Newf(http.StatusMethodNotAllowed, "wanted POST but got %s", r.Method) + 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. + // 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) } @@ -875,7 +877,9 @@ func (h *handlerState) handleAuthCodeCallback(w http.ResponseWriter, r *http.Req } else { // Return HTTP 405 for anything that's not a GET. if r.Method != http.MethodGet { - return httperr.Newf(http.StatusMethodNotAllowed, "wanted GET but got %s", r.Method) + 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. diff --git a/pkg/oidcclient/login_test.go b/pkg/oidcclient/login_test.go index ba4ce3aeb..8c2bfb201 100644 --- a/pkg/oidcclient/login_test.go +++ b/pkg/oidcclient/login_test.go @@ -1753,19 +1753,19 @@ func TestHandleAuthCodeCallback(t *testing.T) { wantHeaders http.Header }{ { - name: "wrong method", - method: http.MethodPost, - query: "", - wantErr: "wanted GET but got POST", - wantHTTPStatus: http.StatusMethodNotAllowed, + name: "wrong method returns an error but keeps listening", + method: http.MethodPost, + query: "", + wantNoCallbacks: true, + wantHTTPStatus: http.StatusMethodNotAllowed, }, { - name: "wrong method for form_post", - method: http.MethodGet, - query: "", - opt: withFormPostMode, - wantErr: "wanted POST but got GET", - wantHTTPStatus: http.StatusMethodNotAllowed, + name: "wrong method for form_post returns an error but keeps listening", + method: http.MethodGet, + query: "", + opt: withFormPostMode, + wantNoCallbacks: true, + wantHTTPStatus: http.StatusMethodNotAllowed, }, { name: "invalid form for form_post", @@ -1970,6 +1970,7 @@ func TestHandleAuthCodeCallback(t *testing.T) { require.Equal(t, tt.wantHeaders, resp.Header()) } + gotCallback := false select { case <-time.After(1 * time.Second): if !tt.wantNoCallbacks { @@ -1983,7 +1984,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) }) } }