Merge main at 0612654 into github_identity_provider

Dockerfile

@@ -3,7 +3,7 @@
 # Copyright 2020-2024 the Pinniped contributors. All Rights Reserved.
 # SPDX-License-Identifier: Apache-2.0
 
-ARG BUILD_IMAGE=golang:1.22.2@sha256:d5302d40dc5fbbf38ec472d1848a9d2391a13f93293a6a5b0b87c99dc0eaa6ae
+ARG BUILD_IMAGE=golang:1.22.3@sha256:b1e05e2c918f52c59d39ce7d5844f73b2f4511f7734add8bb98c9ecdd4443365
 ARG BASE_IMAGE=gcr.io/distroless/static:nonroot@sha256:e9ac71e2b8e279a8372741b7a0293afda17650d926900233ec3a7b2b7c22a246
 
 # Prepare to cross-compile by always running the build stage in the build platform, not the target platform.

hack/Dockerfile_fips

@@ -16,7 +16,7 @@
 # See https://go.googlesource.com/go/+/dev.boringcrypto/README.boringcrypto.md
 # and https://kupczynski.info/posts/fips-golang/ for details.
 
-ARG BUILD_IMAGE=golang:1.22.2@sha256:d5302d40dc5fbbf38ec472d1848a9d2391a13f93293a6a5b0b87c99dc0eaa6ae
+ARG BUILD_IMAGE=golang:1.22.3@sha256:b1e05e2c918f52c59d39ce7d5844f73b2f4511f7734add8bb98c9ecdd4443365
 ARG BASE_IMAGE=gcr.io/distroless/static:nonroot@sha256:e9ac71e2b8e279a8372741b7a0293afda17650d926900233ec3a7b2b7c22a246
 
 # This is not currently using --platform to prepare to cross-compile because we use gcc below to build

pkg/oidcclient/login.go

@@ -173,24 +173,11 @@ func WithScopes(scopes []string) Option {
 	}
 }
 
-// WithBrowserOpen overrides the default "open browser" functionality with a custom callback. If not specified,
-// an implementation using https://github.com/pkg/browser will be used by default.
-//
-// Deprecated: this option will be removed in a future version of Pinniped. See the
-// WithSkipBrowserOpen() option instead.
-func WithBrowserOpen(openURL func(url string) error) Option {
-	return func(h *handlerState) error {
-		h.openURL = openURL
-		return nil
-	}
-}
-
 // WithSkipBrowserOpen causes the login to only print the authorize URL, but skips attempting to
 // open the user's default web browser.
 func WithSkipBrowserOpen() Option {
 	return func(h *handlerState) error {
 		h.skipBrowser = true
-		h.openURL = func(_ string) error { return nil }
 		return nil
 	}
 }
@@ -629,14 +616,13 @@ func (h *handlerState) webBrowserBasedAuth(authorizeOptions *[]oauth2.AuthCodeOp
 
 	// Open the authorize URL in the users browser, logging but otherwise ignoring any error.
 	// Keep track of whether the browser was opened.
-	openedBrowser := true
-	if err := h.openURL(authorizeURL); err != nil {
-		openedBrowser = false
-		h.logger.V(plog.KlogLevelDebug).Error(err, "could not open browser")
-	}
-	if h.skipBrowser {
-		// We didn't actually try to open the browser in the above call to openURL().
-		openedBrowser = false
+	openedBrowser := false
+	if !h.skipBrowser {
+		if err := h.openURL(authorizeURL); err != nil {
+			h.logger.V(plog.KlogLevelDebug).Error(err, "could not open browser")
+		} else {
+			openedBrowser = true
+		}
 	}
 
 	// Allow optionally skipping printing the login URL, for example because printing it may confuse
@@ -669,9 +655,10 @@ func (h *handlerState) webBrowserBasedAuth(authorizeOptions *[]oauth2.AuthCodeOp
 // prompt to the screen and wait for user input, if needed. It can be cancelled by the context provided.
 // It returns a function which should be invoked by the caller to perform some cleanup.
 func (h *handlerState) promptForWebLogin(ctx context.Context, authorizeURL string, printAuthorizeURL bool) func() {
-	if printAuthorizeURL {
-		_, _ = fmt.Fprintf(h.out, "Log in by visiting this link:\n\n    %s\n\n", authorizeURL)
+	if !printAuthorizeURL {
+		return func() {}
 	}
+	_, _ = fmt.Fprintf(h.out, "Log in by visiting this link:\n\n    %s\n\n", authorizeURL)
 
 	// If stdin is not a TTY, don't prompt for the manual paste, since we have no way of reading it.
 	if !h.stdinIsTTY() {

pkg/oidcclient/login_test.go

@@ -712,6 +712,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo
 					h.stdinIsTTY = func() bool { return true }
 					require.NoError(t, WithClient(buildHTTPClientForPEM(formPostSuccessServerCA))(h))
 					require.NoError(t, WithSkipListen()(h))
+					h.skipBrowser = false // don't skip calling the following openURL func
 					h.openURL = func(authorizeURL string) error {
 						parsed, err := url.Parse(authorizeURL)
 						require.NoError(t, err)
@@ -751,6 +752,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo
 					h.stdinIsTTY = func() bool { return true }
 					require.NoError(t, WithClient(buildHTTPClientForPEM(formPostSuccessServerCA))(h))
 					h.listen = func(string, string) (net.Listener, error) { return nil, fmt.Errorf("some listen error") }
+					h.skipBrowser = false // don't skip calling the following openURL func
 					h.openURL = func(authorizeURL string) error {
 						parsed, err := url.Parse(authorizeURL)
 						require.NoError(t, err)
@@ -794,6 +796,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo
 					ctx, cancel := context.WithCancel(h.ctx)
 					h.ctx = ctx
 
+					h.skipBrowser = false // don't skip calling the following openURL func
 					h.openURL = func(_ string) error {
 						cancel()
 						return nil
@@ -824,6 +827,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo
 					h.generateNonce = func() (nonce.Nonce, error) { return "test-nonce", nil }
 					h.stdinIsTTY = func() bool { return true }
 					require.NoError(t, WithClient(buildHTTPClientForPEM(successServerCA))(h))
+					h.skipBrowser = false // don't skip calling the following openURL func
 					h.openURL = func(_ string) error {
 						go func() {
 							h.callbacks <- callbackResult{err: fmt.Errorf("some callback error")}
@@ -876,6 +880,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo
 					client.Timeout = 10 * time.Second
 					require.NoError(t, WithClient(client)(h))
 
+					h.skipBrowser = false // don't skip calling the following openURL func
 					h.openURL = func(actualURL string) error {
 						parsedActualURL, err := url.Parse(actualURL)
 						require.NoError(t, err)
@@ -950,7 +955,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo
 					client.Timeout = 10 * time.Second
 					require.NoError(t, WithClient(client)(h))
 
-					h.skipBrowser = false
+					h.skipBrowser = false // don't skip calling the following openURL func
 					h.openURL = func(actualURL string) error {
 						parsedActualURL, err := url.Parse(actualURL)
 						require.NoError(t, err)
@@ -1016,7 +1021,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo
 					client.Timeout = 10 * time.Second
 					require.NoError(t, WithClient(client)(h))
 
-					h.skipBrowser = false
+					h.skipBrowser = false // don't skip calling the following openURL func
 					h.openURL = func(actualURL string) error {
 						parsedActualURL, err := url.Parse(actualURL)
 						require.NoError(t, err)
@@ -1095,33 +1100,15 @@ func TestLogin(t *testing.T) { //nolint:gocyclo
 					require.NoError(t, WithClient(client)(h))
 
 					h.skipBrowser = true
-					h.openURL = func(actualURL string) error {
-						parsedActualURL, err := url.Parse(actualURL)
-						require.NoError(t, err)
-						actualParams := parsedActualURL.Query()
 
-						require.Contains(t, actualParams.Get("redirect_uri"), "http://127.0.0.1:")
-						actualParams.Del("redirect_uri")
+					// Allow the login to finish so this test does not hang waiting for the callback,
+					// and so we can check if the authorize URL was shown on stderr.
+					// The openURL function will be skipped, so we can't put this code inside the
+					// mock version of that function as we do for other tests in this file.
+					go func() {
+						h.callbacks <- callbackResult{token: &testToken}
+					}()
 
-						require.Equal(t, url.Values{
-							"code_challenge":        []string{testCodeChallenge},
-							"code_challenge_method": []string{"S256"},
-							"response_type":         []string{"code"},
-							"scope":                 []string{"test-scope"},
-							"nonce":                 []string{"test-nonce"},
-							"state":                 []string{"test-state"},
-							"access_type":           []string{"offline"},
-							"client_id":             []string{"test-client-id"},
-						}, actualParams)
-
-						parsedActualURL.RawQuery = ""
-						require.Equal(t, successServer.URL+"/authorize", parsedActualURL.String())
-
-						go func() {
-							h.callbacks <- callbackResult{token: &testToken}
-						}()
-						return nil
-					}
 					return nil
 				}
 			},
@@ -1185,6 +1172,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo
 					client.Timeout = 10 * time.Second
 					require.NoError(t, WithClient(client)(h))
 
+					h.skipBrowser = false // don't skip calling the following openURL func
 					h.openURL = func(actualURL string) error {
 						parsedActualURL, err := url.Parse(actualURL)
 						require.NoError(t, err)
@@ -1261,6 +1249,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo
 					client.Timeout = 10 * time.Second
 					require.NoError(t, WithClient(client)(h))
 
+					h.skipBrowser = false // don't skip calling the following openURL func
 					h.openURL = func(actualURL string) error {
 						parsedActualURL, err := url.Parse(actualURL)
 						require.NoError(t, err)
@@ -2415,7 +2404,7 @@ func TestLogin(t *testing.T) { //nolint:gocyclo
 				WithContext(context.Background()),
 				WithListenPort(0),
 				WithScopes([]string{"test-scope"}),
-				WithSkipBrowserOpen(),
+				WithSkipBrowserOpen(), // Skip by default so we don't really open a browser. Each test can override this.
 				tt.opt(t),
 				WithLogger(testLogger.Logger),
 				withOutWriter(t, &buffer),
@@ -2591,14 +2580,12 @@ func TestHandlePasteCallback(t *testing.T) {
 			},
 		},
 		{
-			name: "success, without printing auth url",
+			name: "skipping printing auth url (also skips prompting for authcode)",
 			opt: func(t *testing.T) Option {
 				return func(h *handlerState) error {
 					h.stdinIsTTY = func() bool { return true }
 					h.useFormPost = true
-					h.promptForValue = func(_ context.Context, promptLabel string, _ io.Writer) (string, error) {
-						return "valid", nil
-					}
+					h.promptForValue = nil // shouldn't get called, so can be nil
 					h.oauth2Config = &oauth2.Config{RedirectURL: testRedirectURI}
 					h.getProvider = func(_ *oauth2.Config, _ *oidc.Provider, _ *http.Client) upstreamprovider.UpstreamOIDCIdentityProviderI {
 						mock := mockUpstream(t)
@@ -2611,11 +2598,8 @@ func TestHandlePasteCallback(t *testing.T) {
 				}
 			},
 			authorizeURL:      testAuthURL,
-			printAuthorizeURL: false,                                 // do not want to print auth URL
-			wantStderr:        newlineAfterEveryAuthcodePromptOutput, // auth URL was not printed to stdout
-			wantCallback: &callbackResult{
-				token: &oidctypes.Token{IDToken: &oidctypes.IDToken{Token: "test-id-token"}},
-			},
+			printAuthorizeURL: false, // do not want to print auth URL
+			wantStderr:        "",    // auth URL was not printed, and prompt for pasting authcode was also not printed
 		},
 	}
 	for _, tt := range tests {

site/config.yaml

@@ -7,8 +7,8 @@ params:
   github_url: "https://github.com/vmware-tanzu/pinniped"
   slack_url: "https://go.pinniped.dev/community/slack"
   community_url: "https://go.pinniped.dev/community"
-  latest_version: v0.29.0
-  latest_codegen_version: 1.29
+  latest_version: v0.30.0
+  latest_codegen_version: 1.30
 pygmentsCodefences: true
 pygmentsStyle: "pygments"
 markup: