Skip to content

Commit e47dac6

Browse files
burmudarburmudar
committed
fix/login: override config endpoint if endpoint positional argument is given (#1300)
* determine if we are using the Default endpoint or not * move notifce of config file deprecation earlier * Move endpoint validation to happen before running loginCmd - update config endpointURL if endpointURL is set either as a flag or positional argument - warn the user about endpoint conflict or that the default endpoint is being used * fix tests and remove unused login funcs * move DefaultEndpointConfigured to be attr on struct * fix tests
1 parent cdb00ca commit e47dac6

5 files changed

Lines changed: 85 additions & 149 deletions

File tree

‎cmd/src/login.go‎

Lines changed: 38 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -50,25 +50,43 @@ Examples:
5050
return err
5151
}
5252

53-
var loginEndpointURL *url.URL
53+
if cfg.configFilePath != "" {
54+
fmt.Fprintln(os.Stderr)
55+
fmt.Fprintf(os.Stderr, "⚠️ 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", cfg.configFilePath)
56+
}
57+
5458
if flagSet.NArg() >= 1 {
5559
arg := flagSet.Arg(0)
56-
u, err := parseEndpoint(arg)
60+
loginEndpointURL, err := parseEndpoint(arg)
5761
if err != nil {
5862
return cmderrors.Usage(fmt.Sprintf("invalid endpoint URL: %s", arg))
5963
}
60-
loginEndpointURL = u
64+
65+
hasEndpointURLConflict := cfg.endpointURL.String() != loginEndpointURL.String()
66+
67+
if hasEndpointURLConflict {
68+
// If the default is configured it means SRC_ENDPOINT is not set
69+
if cfg.usingDefaultEndpoint {
70+
fmt.Fprintf(os.Stderr, "⚠️ Warning: No SRC_ENDPOINT is configured in the environment. Logging in using %q.\n", loginEndpointURL)
71+
fmt.Fprintf(os.Stderr, "\n💡 Tip: To use this endpoint in your shell, run:\n\n export SRC_ENDPOINT=%s\n\nNOTE: By default src will use %q if SRC_ENDPOINT is not set.\n", loginEndpointURL, SGDotComEndpoint)
72+
} else {
73+
fmt.Fprintf(os.Stderr, "⚠️ Warning: Logging into %s instead of the configured endpoint %s.\n", loginEndpointURL, cfg.endpointURL)
74+
fmt.Fprintf(os.Stderr, "\n💡 Tip: To use this endpoint in your shell, run:\n\n export SRC_ENDPOINT=%s\n\n", loginEndpointURL)
75+
}
76+
}
77+
78+
// An explicit endpoint on the CLI overrides the configured endpoint for this login.
79+
cfg.endpointURL = loginEndpointURL
6180
}
6281

6382
client := cfg.apiClient(apiFlags, io.Discard)
6483

6584
return loginCmd(context.Background(), loginParams{
66-
cfg: cfg,
67-
client: client,
68-
out: os.Stdout,
69-
apiFlags: apiFlags,
70-
oauthClient: oauth.NewClient(oauth.DefaultClientID),
71-
loginEndpointURL: loginEndpointURL,
85+
cfg: cfg,
86+
client: client,
87+
out: os.Stdout,
88+
apiFlags: apiFlags,
89+
oauthClient: oauth.NewClient(oauth.DefaultClientID),
7290
})
7391
}
7492

@@ -80,56 +98,34 @@ Examples:
8098
}
8199

82100
type loginParams struct {
83-
cfg *config
84-
client api.Client
85-
out io.Writer
86-
apiFlags *api.Flags
87-
oauthClient oauth.Client
88-
loginEndpointURL *url.URL
101+
cfg *config
102+
client api.Client
103+
out io.Writer
104+
apiFlags *api.Flags
105+
oauthClient oauth.Client
89106
}
90107

91108
type loginFlow func(context.Context, loginParams) error
92109

93-
type loginFlowKind int
94-
95-
const (
96-
loginFlowOAuth loginFlowKind = iota
97-
loginFlowMissingAuth
98-
loginFlowEndpointConflict
99-
loginFlowValidate
100-
)
101-
102110
func loginCmd(ctx context.Context, p loginParams) error {
103111
if err := p.cfg.requireCIAccessToken(); err != nil {
104112
return err
105113
}
106114

107-
if p.cfg.configFilePath != "" {
108-
fmt.Fprintln(p.out)
109-
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)
110-
}
111-
112-
_, flow := selectLoginFlow(p)
115+
flow := selectLoginFlow(p)
113116
if err := flow(ctx, p); err != nil {
114117
return err
115118
}
116-
fmt.Fprintf(p.out, "\n💡 Tip: To use this endpoint in your shell, run:\n\n export SRC_ENDPOINT=%s\n\n", p.cfg.endpointURL)
117119
return nil
118120
}
119121

120122
// selectLoginFlow decides what login flow to run based on configured AuthMode.
121-
func selectLoginFlow(p loginParams) (loginFlowKind, loginFlow) {
122-
if p.loginEndpointURL != nil && p.loginEndpointURL.String() != p.cfg.endpointURL.String() {
123-
return loginFlowEndpointConflict, runEndpointConflictLogin
124-
}
125-
switch p.cfg.AuthMode() {
126-
case AuthModeOAuth:
127-
return loginFlowOAuth, runOAuthLogin
128-
case AuthModeAccessToken:
129-
return loginFlowValidate, runValidatedLogin
130-
default:
131-
return loginFlowMissingAuth, runMissingAuthLogin
123+
124+
func selectLoginFlow(p loginParams) loginFlow {
125+
if p.cfg.AuthMode() == AuthModeAccessToken {
126+
return runValidatedLogin
132127
}
128+
return runOAuthLogin
133129
}
134130

135131
func printLoginProblem(out io.Writer, problem string) {

‎cmd/src/login_test.go‎

Lines changed: 11 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -26,33 +26,22 @@ func mustParseURL(t *testing.T, raw string) *url.URL {
2626
}
2727

2828
func TestLogin(t *testing.T) {
29-
check := func(t *testing.T, cfg *config, endpointArgURL *url.URL) (output string, err error) {
29+
check := func(t *testing.T, cfg *config) (output string, err error) {
3030
t.Helper()
3131

3232
var out bytes.Buffer
3333
err = loginCmd(context.Background(), loginParams{
34-
cfg: cfg,
35-
client: cfg.apiClient(nil, io.Discard),
36-
out: &out,
37-
oauthClient: fakeOAuthClient{startErr: fmt.Errorf("oauth unavailable")},
38-
loginEndpointURL: endpointArgURL,
34+
cfg: cfg,
35+
client: cfg.apiClient(nil, io.Discard),
36+
out: &out,
37+
oauthClient: fakeOAuthClient{startErr: fmt.Errorf("oauth unavailable")},
3938
})
4039
return strings.TrimSpace(out.String()), err
4140
}
4241

43-
t.Run("different endpoint in config vs. arg", func(t *testing.T) {
44-
out, err := check(t, &config{endpointURL: &url.URL{Scheme: "https", Host: "example.com"}}, &url.URL{Scheme: "https", Host: "sourcegraph.example.com"})
45-
if err == nil {
46-
t.Fatal(err)
47-
}
48-
if !strings.Contains(out, "The configured endpoint is https://example.com, not https://sourcegraph.example.com.") {
49-
t.Errorf("got output %q, want configured endpoint error", out)
50-
}
51-
})
52-
5342
t.Run("no access token triggers oauth flow", func(t *testing.T) {
5443
u := &url.URL{Scheme: "https", Host: "example.com"}
55-
out, err := check(t, &config{endpointURL: u}, u)
44+
out, err := check(t, &config{endpointURL: u})
5645
if err == nil {
5746
t.Fatal(err)
5847
}
@@ -63,7 +52,7 @@ func TestLogin(t *testing.T) {
6352

6453
t.Run("CI requires access token", func(t *testing.T) {
6554
u := &url.URL{Scheme: "https", Host: "example.com"}
66-
out, err := check(t, &config{endpointURL: u, inCI: true}, u)
55+
out, err := check(t, &config{endpointURL: u, inCI: true})
6756
if err != errCIAccessTokenRequired {
6857
t.Fatalf("err = %v, want %v", err, errCIAccessTokenRequired)
6958
}
@@ -72,28 +61,14 @@ func TestLogin(t *testing.T) {
7261
}
7362
})
7463

75-
t.Run("warning when using config file", func(t *testing.T) {
76-
endpoint := &url.URL{Scheme: "https", Host: "example.com"}
77-
out, err := check(t, &config{endpointURL: endpoint, configFilePath: "f"}, endpoint)
78-
if err != cmderrors.ExitCode1 {
79-
t.Fatal(err)
80-
}
81-
if !strings.Contains(out, "Configuring src with a JSON file is deprecated") {
82-
t.Errorf("got output %q, want deprecation warning", out)
83-
}
84-
if !strings.Contains(out, "OAuth Device flow authentication failed:") {
85-
t.Errorf("got output %q, want oauth failure output", out)
86-
}
87-
})
88-
8964
t.Run("invalid access token", func(t *testing.T) {
9065
s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
9166
http.Error(w, "", http.StatusUnauthorized)
9267
}))
9368
defer s.Close()
9469

9570
u := mustParseURL(t, s.URL)
96-
out, err := check(t, &config{endpointURL: u, accessToken: "x"}, u)
71+
out, err := check(t, &config{endpointURL: u, accessToken: "x"})
9772
if err != cmderrors.ExitCode1 {
9873
t.Fatal(err)
9974
}
@@ -111,11 +86,11 @@ func TestLogin(t *testing.T) {
11186
defer s.Close()
11287

11388
u := mustParseURL(t, s.URL)
114-
out, err := check(t, &config{endpointURL: u, accessToken: "x"}, u)
89+
out, err := check(t, &config{endpointURL: u, accessToken: "x"})
11590
if err != nil {
11691
t.Fatal(err)
11792
}
118-
wantOut := "✔︎ Authenticated as alice on $ENDPOINT\n\n\n💡 Tip: To use this endpoint in your shell, run:\n\n export SRC_ENDPOINT=$ENDPOINT"
93+
wantOut := "✔︎ Authenticated as alice on $ENDPOINT"
11994
wantOut = strings.ReplaceAll(wantOut, "$ENDPOINT", s.URL)
12095
if out != wantOut {
12196
t.Errorf("got output %q, want %q", out, wantOut)
@@ -156,7 +131,7 @@ func TestLogin(t *testing.T) {
156131
t.Fatal("expected stored oauth token to avoid device flow")
157132
}
158133
gotOut := strings.TrimSpace(out.String())
159-
wantOut := "✔︎ Authenticated as alice on $ENDPOINT\n\n\n✔︎ Authenticated with OAuth credentials\n\n💡 Tip: To use this endpoint in your shell, run:\n\n export SRC_ENDPOINT=$ENDPOINT"
134+
wantOut := "✔︎ Authenticated as alice on $ENDPOINT\n\n\n✔︎ Authenticated with OAuth credentials"
160135
wantOut = strings.ReplaceAll(wantOut, "$ENDPOINT", s.URL)
161136
if gotOut != wantOut {
162137
t.Errorf("got output %q, want %q", gotOut, wantOut)
@@ -192,39 +167,6 @@ func (f fakeOAuthClient) Refresh(context.Context, *oauth.Token) (*oauth.TokenRes
192167
return nil, fmt.Errorf("unexpected call to Refresh")
193168
}
194169

195-
func TestSelectLoginFlow(t *testing.T) {
196-
t.Run("uses oauth flow when no access token is configured", func(t *testing.T) {
197-
params := loginParams{
198-
cfg: &config{endpointURL: mustParseURL(t, "https://example.com")},
199-
}
200-
201-
if got, _ := selectLoginFlow(params); got != loginFlowOAuth {
202-
t.Fatalf("flow = %v, want %v", got, loginFlowOAuth)
203-
}
204-
})
205-
206-
t.Run("uses endpoint conflict flow when auth exists for a different endpoint", func(t *testing.T) {
207-
params := loginParams{
208-
cfg: &config{endpointURL: mustParseURL(t, "https://example.com"), accessToken: "x"},
209-
loginEndpointURL: mustParseURL(t, "https://sourcegraph.example.com"),
210-
}
211-
212-
if got, _ := selectLoginFlow(params); got != loginFlowEndpointConflict {
213-
t.Fatalf("flow = %v, want %v", got, loginFlowEndpointConflict)
214-
}
215-
})
216-
217-
t.Run("uses validation flow when auth exists for the selected endpoint", func(t *testing.T) {
218-
params := loginParams{
219-
cfg: &config{endpointURL: mustParseURL(t, "https://example.com"), accessToken: "x"},
220-
}
221-
222-
if got, _ := selectLoginFlow(params); got != loginFlowValidate {
223-
t.Fatalf("flow = %v, want %v", got, loginFlowValidate)
224-
}
225-
})
226-
}
227-
228170
func TestValidateBrowserURL(t *testing.T) {
229171
tests := []struct {
230172
name string

‎cmd/src/login_validate.go‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,20 +11,6 @@ import (
1111
"github.com/sourcegraph/src-cli/internal/cmderrors"
1212
)
1313

14-
func runMissingAuthLogin(_ context.Context, p loginParams) error {
15-
fmt.Fprintln(p.out)
16-
printLoginProblem(p.out, "No access token is configured.")
17-
fmt.Fprintln(p.out, loginAccessTokenMessage(p.cfg.endpointURL))
18-
return cmderrors.ExitCode1
19-
}
20-
21-
func runEndpointConflictLogin(_ context.Context, p loginParams) error {
22-
fmt.Fprintln(p.out)
23-
printLoginProblem(p.out, fmt.Sprintf("The configured endpoint is %s, not %s.", p.cfg.endpointURL, p.loginEndpointURL))
24-
fmt.Fprintln(p.out, loginAccessTokenMessage(p.loginEndpointURL))
25-
return cmderrors.ExitCode1
26-
}
27-
2814
func runValidatedLogin(ctx context.Context, p loginParams) error {
2915
return validateCurrentUser(ctx, p.client, p.out, p.cfg.endpointURL)
3016
}

‎cmd/src/main.go‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ import (
1919
"github.com/sourcegraph/src-cli/internal/oauth"
2020
)
2121

22+
const SGDotComEndpoint = "https://sourcegraph.com"
23+
2224
const usageText = `src is a tool that provides access to Sourcegraph instances.
2325
For more information, see https://github.com/sourcegraph/src-cli
2426
@@ -131,13 +133,14 @@ var cfg *config
131133

132134
// config holds the resolved configuration used at runtime.
133135
type config struct {
134-
accessToken string
135-
additionalHeaders map[string]string
136-
proxyURL *url.URL
137-
proxyPath string
138-
configFilePath string
139-
endpointURL *url.URL // always non-nil; defaults to https://sourcegraph.com via readConfig
140-
inCI bool
136+
accessToken string
137+
additionalHeaders map[string]string
138+
proxyURL *url.URL
139+
proxyPath string
140+
configFilePath string
141+
endpointURL *url.URL // always non-nil; defaults to https://sourcegraph.com via readConfig
142+
usingDefaultEndpoint bool
143+
inCI bool
141144
}
142145

143146
// configFromFile holds the config as read from the config file,
@@ -260,7 +263,8 @@ func readConfig() (*config, error) {
260263
endpointStr = envEndpoint
261264
}
262265
if endpointStr == "" {
263-
endpointStr = "https://sourcegraph.com"
266+
endpointStr = SGDotComEndpoint
267+
cfg.usingDefaultEndpoint = true
264268
}
265269
if envProxy != "" {
266270
proxyStr = envProxy

0 commit comments

Comments
 (0)