From b6580b303a4f77f310c57027934a5bdb26acc8a2 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Fri, 9 Jul 2021 16:23:15 -0500 Subject: [PATCH 1/2] Reduce CLI callback shutdown timeout (5s -> 500ms). I found that there are some situations with `response_mode=form_post` where Chrome will open additional speculative TCP connections. These connections will be idle so they block server shutdown until the (previously 5s) timeout. Lowering this to 500ms should be safe and makes any added latency at login much less noticeable. More information about Chrome's TCP-level behavior here: https://bugs.chromium.org/p/chromium/issues/detail?id=116982#c5 Signed-off-by: Matt Moyer --- pkg/oidcclient/login.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/oidcclient/login.go b/pkg/oidcclient/login.go index 36b688e2..ffd827e9 100644 --- a/pkg/oidcclient/login.go +++ b/pkg/oidcclient/login.go @@ -850,9 +850,9 @@ func (h *handlerState) serve(listener net.Listener) func() { } go func() { _ = srv.Serve(listener) }() return func() { - // Gracefully shut down the server, allowing up to 5 seconds for + // Gracefully shut down the server, allowing up to 5 00ms for // clients to receive any in-flight responses. - shutdownCtx, cancel := context.WithTimeout(h.ctx, 5*time.Second) + shutdownCtx, cancel := context.WithTimeout(h.ctx, 500*time.Millisecond) _ = srv.Shutdown(shutdownCtx) cancel() } From 5527566a3614189d0a7e9e23279cf28a62719e22 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Fri, 9 Jul 2021 16:26:05 -0500 Subject: [PATCH 2/2] Fix TestCLILoginOIDC when running directly against Okta. Our actual CLI code behaved correctly, but this test made some invalid assumptions about the "upstream" IDP we're testing. It assumed that the upstream didn't support `response_mode=form_post`, but Okta does. This means that when we end up on the localhost callback page, there are no URL query parameters. Adjusting this regex makes the test pass as expected. Signed-off-by: Matt Moyer --- test/integration/cli_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/cli_test.go b/test/integration/cli_test.go index 2e69bc32..ea710e92 100644 --- a/test/integration/cli_test.go +++ b/test/integration/cli_test.go @@ -355,7 +355,7 @@ func runPinnipedLoginOIDC( // Expect to be redirected to the localhost callback. t.Logf("waiting for redirect to callback") - callbackURLPattern := regexp.MustCompile(`\A` + regexp.QuoteMeta(env.CLIUpstreamOIDC.CallbackURL) + `\?.+\z`) + callbackURLPattern := regexp.MustCompile(`\A` + regexp.QuoteMeta(env.CLIUpstreamOIDC.CallbackURL) + `(\?.+)?\z`) browsertest.WaitForURL(t, page, callbackURLPattern) // Wait for the "pre" element that gets rendered for a `text/plain` page, and