refactor: validate and parse the endpoint and proxy at program load (#1267)

* refactor: validate and parse the endpoint and proxy at program load

Amp-Thread-ID: https://ampcode.com/threads/T-019cdb3f-f7de-750b-b4c3-13762c7dfc11
Co-authored-by: Amp <amp@ampcode.com>

* Update internal/oauth/flow.go

Co-authored-by: William Bezuidenhout <william.bezuidenhout@sourcegraph.com>

---------

Co-authored-by: Amp <amp@ampcode.com>
Co-authored-by: William Bezuidenhout <william.bezuidenhout@sourcegraph.com>

Author: 3 people

cmd/src/batch_common.go

@@ -537,7 +537,7 @@ func executeBatchSpec(ctx context.Context, opts executeBatchSpecOpts) (err error
 	if err != nil {
 		return execUI.CreatingBatchSpecError(lr.MaxUnlicensedChangesets, err)
 	}
-	previewURL := cfg.Endpoint + url
+	previewURL := cfg.endpointURL.JoinPath(url).String()
 	execUI.CreatingBatchSpecSuccess(previewURL)
 
 	hasWorkspaceFiles := false
@@ -567,7 +567,7 @@ func executeBatchSpec(ctx context.Context, opts executeBatchSpecOpts) (err error
 	if err != nil {
 		return err
 	}
-	execUI.ApplyingBatchSpecSuccess(cfg.Endpoint + batch.URL)
+	execUI.ApplyingBatchSpecSuccess(cfg.endpointURL.JoinPath(batch.URL).String())
 
 	return nil
 }

cmd/src/batch_remote.go

@@ -5,7 +5,6 @@ import (
 	"flag"
 	"fmt"
 	cliLog "log"
-	"strings"
 	"time"
 
 	"github.com/sourcegraph/sourcegraph/lib/errors"
@@ -155,13 +154,14 @@ Examples:
 		}
 		ui.ExecutingBatchSpecSuccess()
 
-		executionURL := fmt.Sprintf(
-			"%s/%s/batch-changes/%s/executions/%s",
-			strings.TrimSuffix(cfg.Endpoint, "/"),
-			strings.TrimPrefix(namespace.URL, "/"),
-			batchChangeName,
-			batchSpecID,
-		)
+		executionURL := cfg.endpointURL.JoinPath(
+			fmt.Sprintf(
+				"%s/batch-changes/%s/executions/%s",
+				namespace.URL,
+				batchChangeName,
+				batchSpecID,
+			),
+		).String()
 		ui.RemoteSuccess(executionURL)
 
 		return nil

cmd/src/batch_repositories.go

@@ -131,7 +131,7 @@ Examples:
 			Max:                 max,
 			RepoCount:           len(repos),
 			Repos:               repos,
-			SourcegraphEndpoint: cfg.Endpoint,
+			SourcegraphEndpoint: cfg.endpointURL.String(),
 		}); err != nil {
 			return err
 		}

cmd/src/code_intel_upload.go

@@ -7,7 +7,6 @@ import (
 	"flag"
 	"fmt"
 	"io"
-	"net/url"
 	"os"
 	"strings"
 	"time"
@@ -87,10 +86,7 @@ func handleCodeIntelUpload(args []string) error {
 		return handleUploadError(uploadOptions.SourcegraphInstanceOptions.AccessToken, err)
 	}
 
-	uploadURL, err := makeCodeIntelUploadURL(uploadID)
-	if err != nil {
-		return err
-	}
+	uploadURL := makeCodeIntelUploadURL(uploadID)
 
 	if codeintelUploadFlags.json {
 		serialized, err := json.Marshal(map[string]any{
@@ -132,7 +128,7 @@ func codeintelUploadOptions(out *output.Output) upload.UploadOptions {
 		associatedIndexID = &codeintelUploadFlags.associatedIndexID
 	}
 
-	cfg.AdditionalHeaders["Content-Type"] = "application/x-protobuf+scip"
+	cfg.additionalHeaders["Content-Type"] = "application/x-protobuf+scip"
 
 	logger := upload.NewRequestLogger(
 		os.Stdout,
@@ -153,9 +149,9 @@ func codeintelUploadOptions(out *output.Output) upload.UploadOptions {
 			AssociatedIndexID: associatedIndexID,
 		},
 		SourcegraphInstanceOptions: upload.SourcegraphInstanceOptions{
-			SourcegraphURL:      cfg.Endpoint,
-			AccessToken:         cfg.AccessToken,
-			AdditionalHeaders:   cfg.AdditionalHeaders,
+			SourcegraphURL:      cfg.endpointURL.String(),
+			AccessToken:         cfg.accessToken,
+			AdditionalHeaders:   cfg.additionalHeaders,
 			MaxRetries:          5,
 			RetryInterval:       time.Second,
 			Path:                codeintelUploadFlags.uploadRoute,
@@ -191,16 +187,12 @@ func printInferredArguments(out *output.Output) {
 
 // makeCodeIntelUploadURL constructs a URL to the upload with the given internal identifier.
 // The base of the URL is constructed from the configured Sourcegraph instance.
-func makeCodeIntelUploadURL(uploadID int) (string, error) {
-	url, err := url.Parse(cfg.Endpoint)
-	if err != nil {
-		return "", err
-	}
-
+func makeCodeIntelUploadURL(uploadID int) string {
+	// Careful: copy by dereference makes a shallow copy, so User is not duplicated.
+	url := *cfg.endpointURL
 	graphqlID := base64.URLEncoding.EncodeToString(fmt.Appendf(nil, `SCIPUpload:%d`, uploadID))
 	url.Path = codeintelUploadFlags.repo + "/-/code-intelligence/uploads/" + graphqlID
-	url.User = nil
-	return url.String(), nil
+	return url.String()
 }
 
 type errorWithHint struct {

cmd/src/debug_compose.go

@@ -75,7 +75,7 @@ Examples:
 			return errors.Wrap(err, "failed to get containers for subcommand with err")
 		}
 		// Safety check user knows what they are targeting with this debug command
-		log.Printf("This command will archive docker-cli data for %d containers\n SRC_ENDPOINT: %v\n Output filename: %v", len(containers), cfg.Endpoint, base)
+		log.Printf("This command will archive docker-cli data for %d containers\n SRC_ENDPOINT: %v\n Output filename: %v", len(containers), cfg.endpointURL, base)
 		if verified, _ := verify("Do you want to start writing to an archive?"); !verified {
 			return nil
 		}

cmd/src/debug_kube.go

@@ -84,7 +84,7 @@ Examples:
 			return errors.Wrapf(err, "failed to get current-context")
 		}
 		// Safety check user knows what they've targeted with this command
-		log.Printf("Archiving kubectl data for %d pods\n SRC_ENDPOINT: %v\n Context: %s Namespace: %v\n Output filename: %v", len(pods.Items), cfg.Endpoint, kubectx, namespace, base)
+		log.Printf("Archiving kubectl data for %d pods\n SRC_ENDPOINT: %v\n Context: %s Namespace: %v\n Output filename: %v", len(pods.Items), cfg.endpointURL, kubectx, namespace, base)
 		if verified, _ := verify("Do you want to start writing to an archive?"); !verified {
 			return nil
 		}

cmd/src/debug_server.go

@@ -72,7 +72,7 @@ Examples:
 		defer zw.Close()
 
 		// Safety check user knows what they are targeting with this debug command
-		log.Printf("This command will archive docker-cli data for container: %s\n SRC_ENDPOINT: %s\n Output filename: %s", container, cfg.Endpoint, base)
+		log.Printf("This command will archive docker-cli data for container: %s\n SRC_ENDPOINT: %s\n Output filename: %s", container, cfg.endpointURL, base)
 		if verified, _ := verify("Do you want to start writing to an archive?"); !verified {
 			return nil
 		}

cmd/src/login.go

@@ -5,6 +5,7 @@ import (
 	"flag"
 	"fmt"
 	"io"
+	"net/url"
 	"os"
 
 	"github.com/sourcegraph/src-cli/internal/api"
@@ -48,23 +49,26 @@ Examples:
 		if err := flagSet.Parse(args); err != nil {
 			return err
 		}
-		endpoint := cfg.Endpoint
+
+		var loginEndpointURL *url.URL
 		if flagSet.NArg() >= 1 {
-			endpoint = flagSet.Arg(0)
-		}
-		if endpoint == "" {
-			return cmderrors.Usage("expected exactly one argument: the Sourcegraph URL, or SRC_ENDPOINT to be set")
+			arg := flagSet.Arg(0)
+			u, err := parseEndpoint(arg)
+			if err != nil {
+				return cmderrors.Usage(fmt.Sprintf("invalid endpoint URL: %s", arg))
+			}
+			loginEndpointURL = u
 		}
 
 		client := cfg.apiClient(apiFlags, io.Discard)
 
 		return loginCmd(context.Background(), loginParams{
-			cfg:         cfg,
-			client:      client,
-			endpoint:    endpoint,
-			out:         os.Stdout,
-			apiFlags:    apiFlags,
-			oauthClient: oauth.NewClient(oauth.DefaultClientID),
+			cfg:              cfg,
+			client:           client,
+			out:              os.Stdout,
+			apiFlags:         apiFlags,
+			oauthClient:      oauth.NewClient(oauth.DefaultClientID),
+			loginEndpointURL: loginEndpointURL,
 		})
 	}
 
@@ -76,12 +80,12 @@ Examples:
 }
 
 type loginParams struct {
-	cfg         *config
-	client      api.Client
-	endpoint    string
-	out         io.Writer
-	apiFlags    *api.Flags
-	oauthClient oauth.Client
+	cfg              *config
+	client           api.Client
+	out              io.Writer
+	apiFlags         *api.Flags
+	oauthClient      oauth.Client
+	loginEndpointURL *url.URL
 }
 
 type loginFlow func(context.Context, loginParams) error
@@ -96,9 +100,9 @@ const (
 )
 
 func loginCmd(ctx context.Context, p loginParams) error {
-	if p.cfg.ConfigFilePath != "" {
+	if p.cfg.configFilePath != "" {
 		fmt.Fprintln(p.out)
-		fmt.Fprintf(p.out, "⚠️  Warning: Configuring src with a JSON file is deprecated. Please migrate to using the env vars SRC_ENDPOINT, SRC_ACCESS_TOKEN, and SRC_PROXY instead, and then remove %s. See https://github.com/sourcegraph/src-cli#readme for more information.\n", p.cfg.ConfigFilePath)
+		fmt.Fprintf(p.out, "⚠️  Warning: Configuring src with a JSON file is deprecated. Please migrate to using the env vars SRC_ENDPOINT, SRC_ACCESS_TOKEN, and SRC_PROXY instead, and then remove %s. See https://github.com/sourcegraph/src-cli#readme for more information.\n", p.cfg.configFilePath)
 	}
 
 	_, flow := selectLoginFlow(p)
@@ -107,15 +111,13 @@ func loginCmd(ctx context.Context, p loginParams) error {
 
 // selectLoginFlow decides what login flow to run based on configured AuthMode.
 func selectLoginFlow(p loginParams) (loginFlowKind, loginFlow) {
-	endpointArg := cleanEndpoint(p.endpoint)
-
+	if p.loginEndpointURL != nil && p.loginEndpointURL.String() != p.cfg.endpointURL.String() {
+		return loginFlowEndpointConflict, runEndpointConflictLogin
+	}
 	switch p.cfg.AuthMode() {
 	case AuthModeOAuth:
 		return loginFlowOAuth, runOAuthLogin
 	case AuthModeAccessToken:
-		if endpointArg != p.cfg.Endpoint {
-			return loginFlowEndpointConflict, runEndpointConflictLogin
-		}
 		return loginFlowValidate, runValidatedLogin
 	default:
 		return loginFlowMissingAuth, runMissingAuthLogin
@@ -126,7 +128,7 @@ func printLoginProblem(out io.Writer, problem string) {
 	fmt.Fprintf(out, "❌ Problem: %s\n", problem)
 }
 
-func loginAccessTokenMessage(endpoint string) string {
+func loginAccessTokenMessage(endpointURL *url.URL) string {
 	return fmt.Sprintf("\n"+`🛠  To fix: Create an access token by going to %s/user/settings/tokens, then set the following environment variables in your terminal:
 
    export SRC_ENDPOINT=%s
@@ -135,5 +137,5 @@ func loginAccessTokenMessage(endpoint string) string {
    To verify that it's working, run the login command again.
 
    Alternatively, you can try logging in interactively by running: src login %s
-`, endpoint, endpoint, endpoint)
+`, endpointURL, endpointURL, endpointURL)
 }

cmd/src/login_oauth.go

@@ -18,15 +18,14 @@ import (
 var loadStoredOAuthToken = oauth.LoadToken
 
 func runOAuthLogin(ctx context.Context, p loginParams) error {
-	endpointArg := cleanEndpoint(p.endpoint)
-	client, err := oauthLoginClient(ctx, p, endpointArg)
+	client, err := oauthLoginClient(ctx, p)
 	if err != nil {
 		printLoginProblem(p.out, fmt.Sprintf("OAuth Device flow authentication failed: %s", err))
-		fmt.Fprintln(p.out, loginAccessTokenMessage(endpointArg))
+		fmt.Fprintln(p.out, loginAccessTokenMessage(p.cfg.endpointURL))
 		return cmderrors.ExitCode1
 	}
 
-	if err := validateCurrentUser(ctx, client, p.out, endpointArg); err != nil {
+	if err := validateCurrentUser(ctx, client, p.out, p.cfg.endpointURL); err != nil {
 		return err
 	}
 
@@ -39,13 +38,13 @@ func runOAuthLogin(ctx context.Context, p loginParams) error {
 // 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) {
+func oauthLoginClient(ctx context.Context, p loginParams) (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
+	if token, err := loadStoredOAuthToken(ctx, p.cfg.endpointURL); err == nil {
+		return newOAuthAPIClient(p, token), nil
 	}
 
-	token, err := runOAuthDeviceFlow(ctx, endpoint, p.out, p.oauthClient)
+	token, err := runOAuthDeviceFlow(ctx, p.cfg.endpointURL, p.out, p.oauthClient)
 	if err != nil {
 		return nil, err
 	}
@@ -55,23 +54,23 @@ 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
+	return newOAuthAPIClient(p, token), nil
 }
 
-func newOAuthAPIClient(p loginParams, endpoint string, token *oauth.Token) api.Client {
+func newOAuthAPIClient(p loginParams, token *oauth.Token) api.Client {
 	return api.NewClient(api.ClientOpts{
-		Endpoint:          endpoint,
-		AdditionalHeaders: p.cfg.AdditionalHeaders,
+		EndpointURL:       p.cfg.endpointURL,
+		AdditionalHeaders: p.cfg.additionalHeaders,
 		Flags:             p.apiFlags,
 		Out:               p.out,
-		ProxyURL:          p.cfg.ProxyURL,
-		ProxyPath:         p.cfg.ProxyPath,
+		ProxyURL:          p.cfg.proxyURL,
+		ProxyPath:         p.cfg.proxyPath,
 		OAuthToken:        token,
 	})
 }
 
-func runOAuthDeviceFlow(ctx context.Context, endpoint string, out io.Writer, client oauth.Client) (*oauth.Token, error) {
-	authResp, err := client.Start(ctx, endpoint, nil)
+func runOAuthDeviceFlow(ctx context.Context, endpointURL *url.URL, out io.Writer, client oauth.Client) (*oauth.Token, error) {
+	authResp, err := client.Start(ctx, endpointURL, nil)
 	if err != nil {
 		return nil, err
 	}
@@ -95,12 +94,12 @@ func runOAuthDeviceFlow(ctx context.Context, endpoint string, out io.Writer, cli
 		interval = 5 * time.Second
 	}
 
-	resp, err := client.Poll(ctx, endpoint, authResp.DeviceCode, interval, authResp.ExpiresIn)
+	resp, err := client.Poll(ctx, endpointURL, authResp.DeviceCode, interval, authResp.ExpiresIn)
 	if err != nil {
 		return nil, err
 	}
 
-	token := resp.Token(endpoint)
+	token := resp.Token(endpointURL)
 	token.ClientID = client.ClientID()
 	return token, nil
 }