check for OAuth token precense before starting a new OAuth flow

Author: burmudar

cmd/src/login.go

@@ -105,7 +105,7 @@ func loginCmd(ctx context.Context, p loginParams) error {
 	return flow(ctx, p)
 }
 
-// selectLoginFlow decides what login flow to run based on configigured AuthMode.
+// selectLoginFlow decides what login flow to run based on configured AuthMode.
 func selectLoginFlow(_ context.Context, p loginParams) (loginFlowKind, loginFlow) {
 	endpointArg := cleanEndpoint(p.endpoint)
 

cmd/src/login_oauth.go

@@ -13,6 +13,8 @@ import (
 	"github.com/sourcegraph/src-cli/internal/oauth"
 )
 
+var loadStoredOAuthToken = oauth.LoadToken
+
 func runOAuthLogin(ctx context.Context, p loginParams) error {
 	endpointArg := cleanEndpoint(p.endpoint)
 	client, err := oauthLoginClient(ctx, p, endpointArg)
@@ -32,7 +34,15 @@ func runOAuthLogin(ctx context.Context, p loginParams) error {
 	return nil
 }
 
+// oauthLoginClient returns a api.Client with the OAuth token set. It will check secret storage for a token
+// and use it if one is present.
+// If no token is found, it will start a OAuth Device flow to get a token and storage in secret storage.
 func oauthLoginClient(ctx context.Context, p loginParams, endpoint string) (api.Client, error) {
+	// if we have a stored token, used it. Otherwise run the device flow
+	if token, err := loadStoredOAuthToken(ctx, endpoint); err == nil {
+		return newOAuthAPIClient(p, endpoint, token), nil
+	}
+
 	token, err := runOAuthDeviceFlow(ctx, endpoint, p.out, p.oauthClient)
 	if err != nil {
 		return nil, err
@@ -43,6 +53,10 @@ func oauthLoginClient(ctx context.Context, p loginParams, endpoint string) (api.
 		fmt.Fprintf(p.out, "⚠️  Warning: Failed to store token in keyring store: %q. Continuing with this session only.\n", err)
 	}
 
+	return newOAuthAPIClient(p, endpoint, token), nil
+}
+
+func newOAuthAPIClient(p loginParams, endpoint string, token *oauth.Token) api.Client {
 	return api.NewClient(api.ClientOpts{
 		Endpoint:          endpoint,
 		AdditionalHeaders: p.cfg.AdditionalHeaders,
@@ -51,7 +65,7 @@ func oauthLoginClient(ctx context.Context, p loginParams, endpoint string) (api.
 		ProxyURL:          p.cfg.ProxyURL,
 		ProxyPath:         p.cfg.ProxyPath,
 		OAuthToken:        token,
-	}), nil
+	})
 }
 
 func runOAuthDeviceFlow(ctx context.Context, endpoint string, out io.Writer, client oauth.Client) (*oauth.Token, error) {

cmd/src/login_test.go

@@ -98,10 +98,52 @@ func TestLogin(t *testing.T) {
 			t.Errorf("got output %q, want %q", out, wantOut)
 		}
 	})
+
+	t.Run("reuses stored oauth token before device flow", func(t *testing.T) {
+		s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			fmt.Fprintln(w, `{"data":{"currentUser":{"username":"alice"}}}`)
+		}))
+		defer s.Close()
+
+		restoreStoredOAuthLoader(t, func(context.Context, string) (*oauth.Token, error) {
+			return &oauth.Token{
+				Endpoint:    s.URL,
+				ClientID:    oauth.DefaultClientID,
+				AccessToken: "oauth-token",
+				ExpiresAt:   time.Now().Add(time.Hour),
+			}, nil
+		})
+
+		startCalled := false
+		var out bytes.Buffer
+		err := loginCmd(context.Background(), loginParams{
+			cfg:      &config{Endpoint: s.URL},
+			client:   (&config{Endpoint: s.URL}).apiClient(nil, io.Discard),
+			endpoint: s.URL,
+			out:      &out,
+			oauthClient: fakeOAuthClient{
+				startErr:    fmt.Errorf("unexpected call to Start"),
+				startCalled: &startCalled,
+			},
+		})
+		if err != nil {
+			t.Fatal(err)
+		}
+		if startCalled {
+			t.Fatal("expected stored oauth token to avoid device flow")
+		}
+		gotOut := strings.TrimSpace(out.String())
+		wantOut := "✔︎ Authenticated as alice on $ENDPOINT\n\n\n✔︎ Authenticated with OAuth credentials"
+		wantOut = strings.ReplaceAll(wantOut, "$ENDPOINT", s.URL)
+		if gotOut != wantOut {
+			t.Errorf("got output %q, want %q", gotOut, wantOut)
+		}
+	})
 }
 
 type fakeOAuthClient struct {
-	startErr error
+	startErr    error
+	startCalled *bool
 }
 
 func (f fakeOAuthClient) ClientID() string {
@@ -113,6 +155,9 @@ func (f fakeOAuthClient) Discover(context.Context, string) (*oauth.OIDCConfigura
 }
 
 func (f fakeOAuthClient) Start(context.Context, string, []string) (*oauth.DeviceAuthResponse, error) {
+	if f.startCalled != nil {
+		*f.startCalled = true
+	}
 	return nil, f.startErr
 }
 
@@ -158,3 +203,13 @@ func TestSelectLoginFlow(t *testing.T) {
 		}
 	})
 }
+
+func restoreStoredOAuthLoader(t *testing.T, loader func(context.Context, string) (*oauth.Token, error)) {
+	t.Helper()
+
+	prev := loadStoredOAuthToken
+	loadStoredOAuthToken = loader
+	t.Cleanup(func() {
+		loadStoredOAuthToken = prev
+	})
+}