Skip to content

Commit 6cfbd76

Browse files
celebdorclaude
andcommitted
feat(login): extend --auto-open-browser to work with --web
Extend the existing --auto-open-browser flag to support web login in addition to OIDC login. This provides a consistent interface for controlling browser behavior across both authentication methods. Changes: - Extend --auto-open-browser flag to work with both --web and --exec-plugin - Update flag documentation to clarify default behavior (true for --web, false for OIDC) - Set default to true when using --web (unless explicitly overridden) - Remove --auto-open-browser from OIDC-only validation checks - Update login examples to show --auto-open-browser=false usage - Update test coverage to reflect new behavior When --web is used with --auto-open-browser=false, the authorization URL is printed instead of being automatically opened in a browser. This is useful for environments where browser automation is not desired or possible. Fixes: CNTRLPLANE-1538 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent a25e854 commit 6cfbd76

File tree

3 files changed

+157
-4
lines changed

3 files changed

+157
-4
lines changed

pkg/cli/login/login.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,9 @@ var (
4747
# Log in to the given server through a browser
4848
oc login localhost:8443 --web --callback-port 8280
4949
50+
# Log in to the given server through a browser without opening it automatically (print URL only)
51+
oc login localhost:8443 --web --auto-open-browser=false --callback-port 8280
52+
5053
# Log in to the external OIDC issuer through Auth Code + PKCE by starting a local server listening on port 8080
5154
oc login localhost:8443 --exec-plugin=oc-oidc --client-id=client-id --extra-scopes=email,profile --callback-port=8080
5255
`)
@@ -98,7 +101,7 @@ func NewCmdLogin(f kcmdutil.Factory, streams genericiooptions.IOStreams) *cobra.
98101
cmds.Flags().StringSliceVar(&o.OIDCExtraScopes, "extra-scopes", o.OIDCExtraScopes, "Experimental: Extra scopes for external OIDC issuer. Optional.")
99102
cmds.Flags().StringVar(&o.OIDCIssuerURL, "issuer-url", o.OIDCIssuerURL, "Experimental: Issuer url for external issuer. Required.")
100103
cmds.Flags().StringVar(&o.OIDCCAFile, "oidc-certificate-authority", o.OIDCCAFile, "Experimental: The path to a certificate authority bundle to use when communicating with external OIDC issuer.")
101-
cmds.Flags().BoolVar(&o.OIDCAutoOpenBrowser, "auto-open-browser", o.OIDCAutoOpenBrowser, "Experimental: Specify browser is automatically opened or not for external OIDC issuer. Disabled by default.")
104+
cmds.Flags().BoolVar(&o.OIDCAutoOpenBrowser, "auto-open-browser", o.OIDCAutoOpenBrowser, "Automatically open browser for login. When used with --web, defaults to true. When used with --exec-plugin for external OIDC, defaults to false.")
102105
return cmds
103106
}
104107

@@ -120,6 +123,11 @@ func (o *LoginOptions) Complete(f kcmdutil.Factory, cmd *cobra.Command, args []s
120123
}
121124
o.RequestTimeout = timeout
122125

126+
// Set default value for auto-open-browser when using web login
127+
if o.WebLogin && !cmd.Flags().Changed("auto-open-browser") {
128+
o.OIDCAutoOpenBrowser = true
129+
}
130+
123131
parsedDefaultClusterURL, err := url.Parse(defaultClusterURL)
124132
if err != nil {
125133
return err
@@ -191,7 +199,7 @@ func (o LoginOptions) Validate(cmd *cobra.Command, serverFlag string, args []str
191199
return errors.New("currently only oc-oidc is supported")
192200
}
193201

194-
oidcOptionsSet := o.OIDCClientID != "" || o.OIDCClientSecret != "" || len(o.OIDCExtraScopes) > 0 || o.OIDCIssuerURL != "" || o.OIDCAutoOpenBrowser
202+
oidcOptionsSet := o.OIDCClientID != "" || o.OIDCClientSecret != "" || len(o.OIDCExtraScopes) > 0 || o.OIDCIssuerURL != ""
195203

196204
if o.OIDCExecPluginType == "" && oidcOptionsSet {
197205
return errors.New("please specify --exec-plugin type. Currently only oc-oidc is supported")

pkg/cli/login/loginoptions.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -317,8 +317,14 @@ func (o *LoginOptions) gatherAuthInfo() error {
317317
if o.WebLogin {
318318
loginURLHandler := func(u *url.URL) error {
319319
loginURL := u.String()
320-
fmt.Fprintf(o.Out, "Opening login URL in the default browser: %s\n", loginURL)
321-
return browser.OpenURL(loginURL)
320+
if !o.OIDCAutoOpenBrowser {
321+
fmt.Fprintf(o.Out, "Please visit the following URL in your browser: %s\n", loginURL)
322+
fmt.Fprintf(o.Out, "The callback server is listening and will receive the authentication response.\n")
323+
return nil
324+
} else {
325+
fmt.Fprintf(o.Out, "Opening login URL in the default browser: %s\n", loginURL)
326+
return browser.OpenURL(loginURL)
327+
}
322328
}
323329
token, err = tokenrequest.RequestTokenWithLocalCallback(o.Config, loginURLHandler, int(o.CallbackPort))
324330
} else {

pkg/cli/login/loginoptions_test.go

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,16 @@
11
package login
22

33
import (
4+
"bytes"
45
"crypto/tls"
56
"encoding/json"
67
"encoding/pem"
78
"fmt"
89
"net/http"
910
"net/http/httptest"
11+
"net/url"
1012
"regexp"
13+
"strings"
1114
"testing"
1215

1316
"github.com/MakeNowJust/heredoc"
@@ -485,6 +488,142 @@ func TestPreserveExecProviderOnUsernameLogin(t *testing.T) {
485488
}
486489
}
487490

491+
func TestValidateAutoOpenBrowser(t *testing.T) {
492+
testCases := []struct {
493+
name string
494+
webLogin bool
495+
autoOpenBrowser bool
496+
expectedError string
497+
}{
498+
{
499+
name: "valid: --web with --auto-open-browser",
500+
webLogin: true,
501+
autoOpenBrowser: true,
502+
expectedError: "",
503+
},
504+
{
505+
name: "valid: --web with --auto-open-browser=false",
506+
webLogin: true,
507+
autoOpenBrowser: false,
508+
expectedError: "",
509+
},
510+
{
511+
name: "valid: --web without --auto-open-browser (will default to true)",
512+
webLogin: true,
513+
autoOpenBrowser: false,
514+
expectedError: "",
515+
},
516+
{
517+
name: "valid: neither --web nor --auto-open-browser",
518+
webLogin: false,
519+
autoOpenBrowser: false,
520+
expectedError: "",
521+
},
522+
}
523+
524+
for _, tc := range testCases {
525+
t.Run(tc.name, func(t *testing.T) {
526+
options := &LoginOptions{
527+
Server: "https://api.test.devcluster.openshift.com:6443", // Consistent with existing OpenShift tests
528+
WebLogin: tc.webLogin,
529+
OIDCAutoOpenBrowser: tc.autoOpenBrowser,
530+
StartingKubeConfig: &kclientcmdapi.Config{},
531+
}
532+
533+
err := options.Validate(nil, "", []string{})
534+
if tc.expectedError == "" {
535+
if err != nil {
536+
t.Errorf("expected no error, but got: %v", err)
537+
}
538+
} else {
539+
if err == nil {
540+
t.Errorf("expected error '%s', but got no error", tc.expectedError)
541+
} else if err.Error() != tc.expectedError {
542+
t.Errorf("expected error '%s', but got: %v", tc.expectedError, err)
543+
}
544+
}
545+
})
546+
}
547+
}
548+
549+
func TestAutoOpenBrowserURLHandling(t *testing.T) {
550+
testCases := []struct {
551+
name string
552+
autoOpenBrowser bool
553+
expectedOutputRegex string
554+
shouldNotContain string
555+
}{
556+
{
557+
name: "with --auto-open-browser=false prints URL without opening",
558+
autoOpenBrowser: false,
559+
expectedOutputRegex: "Please visit the following URL in your browser: https://example.com/oauth/authorize\\?test=1\nThe callback server is listening and will receive the authentication response.\n",
560+
shouldNotContain: "Opening login URL in the default browser",
561+
},
562+
{
563+
name: "with --auto-open-browser=true shows opening message",
564+
autoOpenBrowser: true,
565+
expectedOutputRegex: "Opening login URL in the default browser: https://example.com/oauth/authorize\\?test=1\n",
566+
shouldNotContain: "Please visit the following URL",
567+
},
568+
}
569+
570+
for _, tc := range testCases {
571+
t.Run(tc.name, func(t *testing.T) {
572+
// Capture output
573+
outBuf := &bytes.Buffer{}
574+
streams := genericiooptions.IOStreams{
575+
In: strings.NewReader(""),
576+
Out: outBuf,
577+
ErrOut: &bytes.Buffer{},
578+
}
579+
580+
options := &LoginOptions{
581+
WebLogin: true,
582+
OIDCAutoOpenBrowser: tc.autoOpenBrowser,
583+
IOStreams: streams,
584+
}
585+
586+
// Create a test login URL handler that matches the actual implementation
587+
loginURLHandler := func(u *url.URL) error {
588+
loginURL := u.String()
589+
if !options.OIDCAutoOpenBrowser {
590+
fmt.Fprintf(options.Out, "Please visit the following URL in your browser: %s\n", loginURL)
591+
fmt.Fprintf(options.Out, "The callback server is listening and will receive the authentication response.\n")
592+
return nil
593+
} else {
594+
fmt.Fprintf(options.Out, "Opening login URL in the default browser: %s\n", loginURL)
595+
// Don't actually call browser.OpenURL in tests
596+
return nil
597+
}
598+
}
599+
600+
// Test the handler with a test URL
601+
testURL, _ := url.Parse("https://example.com/oauth/authorize?test=1")
602+
err := loginURLHandler(testURL)
603+
604+
if err != nil {
605+
t.Errorf("unexpected error: %v", err)
606+
}
607+
608+
output := outBuf.String()
609+
610+
// Check expected output using regex
611+
matched, regexErr := regexp.MatchString(tc.expectedOutputRegex, output)
612+
if regexErr != nil {
613+
t.Fatalf("regex error: %v", regexErr)
614+
}
615+
if !matched {
616+
t.Errorf("output did not match expected pattern.\nExpected pattern: %s\nActual output: %s", tc.expectedOutputRegex, output)
617+
}
618+
619+
// Check that certain strings are not present
620+
if tc.shouldNotContain != "" && strings.Contains(output, tc.shouldNotContain) {
621+
t.Errorf("output should not contain '%s', but got: %s", tc.shouldNotContain, output)
622+
}
623+
})
624+
}
625+
}
626+
488627
func newTLSServer(certString, keyString string) (*httptest.Server, error) {
489628
invoked := make(chan struct{}, 1)
490629
server := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

0 commit comments

Comments
 (0)