login: skip unreachable legacy entry for non-default --server, redact URL errors

f94a1da→main·

toothbrush·1mo ago·2 files·+77 added/-26 removed

Address PR #1404 review comments and the broken login integration tests:

- Every legacy-keyring read keys by api.AuthBaseURL(), which is always the default origin now — a legacy entry saved under a non-default --server origin was unreadable forever and undeletable by logout (Bugbot's logout finding, fixed at the write side instead). For a non-default server the context is the sole record of the login, so RecordLoginContext failure becomes fatal there instead of a warning. - parseLoginServer errors now echo u.Redacted() rather than the raw flag value, so a password in a rejected userinfo URL can't land in CI logs. - The login integration tests still exported the retired ENTIRE_AUTH_BASE_URL (rejected at startup since 6e40fcaa432) and served the v1 device-code path the default split-host provider no longer uses. They now pass --server, serve /device_authorization, mint an iss-bearing fake JWT (context recording needs claims), and sandbox ENTIRE_CONFIG_DIR/token store so the spawned binary can't touch the real ~/.config/entire or OS keychain.

Co-Authored-By: Claude Fable 5 noreply@anthropic.com

Sessions

6849b1e6a444View transcript

Changes

2

• cmd/entire/cli

• integration_test

• Mlogin_test.go+49/-11

• Mlogin.go+28/-15

4 unmodified lines

5
6
7
8
9
10
11
1 unmodified line

13
14
15
16
17
18
19
4 unmodified lines

24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
6 unmodified lines

49
50
51
37
52
53
54
55
13 unmodified lines

69
70
71
57
72
73
74
75
40 unmodified lines

116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
7 unmodified lines

141
142
143
117
144
145
146
147
32 unmodified lines

180
181
182
156
183
184
185
186
48 unmodified lines

235
236
237
211
238
239
240
241
31 unmodified lines

273
274
275
249
276
277
278
279
280
281
282
7 unmodified lines

290
291
292
293
294
295
296
297
298
299
300
301
1 unmodified line

303
304
305
271
272
273
274
306
307
308
309
310
311
312
313
314
315
18 unmodified lines

334
335
336
300
337
338
339
340
341

4 unmodified lines

import (
    "bufio"
    "context"
    "encoding/base64"
    "encoding/json"
    "errors"
    "fmt"
1 unmodified line

"net/http"
    "net/http/httptest"
    "net/url"
    "os"
    "path/filepath"
    "strings"
    "sync"
    
    "github.com/entireio/cli/cmd/entire/cli/testutil"
)

// fakeLoginJWT builds a JWT-shaped access token with a junk signature
// (ParseClaims doesn't verify signatures) whose iss matches the test server
// origin, so login's iss cross-check passes and the context can be recorded.
// A bare opaque token is no longer enough for a --server login: with no iss
// claim there is nothing to key the login context by, and login fails.
func fakeLoginJWT(iss string) string {
    enc := base64.RawURLEncoding
    header := enc.EncodeToString([]byte(`{"alg":"RS256","typ":"JWT"}`))
    payload := enc.EncodeToString(fmt.Appendf(nil,
        `{"iss":%q,"sub":"user-123","exp":%d}", iss, time.Now().Add(time.Hour).Unix()))
    return header + "." + payload + "." + enc.EncodeToString([]byte("sig"))
}

func TestLogin_SavesTokenAfterApproval(t *testing.T) {
    t.Parallel()

serverState := &state{}
    server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        
        switch {
        case r.Method == http.MethodPost && r.URL.Path == "/oauth/device/code":
        case r.Method == http.MethodPost && r.URL.Path == "/device_authorization":
            writeJSON(t, w, http.StatusOK, map[string]any{
                "device_code":               "device-123",
                "user_code":                 "ABCD-EFGH",
                return
            }
            
            writeJSON(t, w, http.StatusOK, map[string]any{"access_token": "local-token", "token_type": "Bearer", "expires_in": 3600, "scope": "cli"})
            writeJSON(t, w, http.StatusOK, map[string]any{"access_token": fakeLoginJWT("http://" + r.Host), "token_type": "Bearer", "expires_in": 3600, "scope": "cli"})
        case r.Method == http.MethodPost && r.URL.Path == "/approve":
            serverState.Lock()
            serverState.approved = true

t.Fatalf("output missing login complete message (token save likely failed):\n%s", output)
    }

// A --server login is recorded as a contexts.json context (the legacy
    // keyring entry is only written for the default login server, whose key
    // is the only one legacy readers consult).
    contextsPath := filepath.Join(proc.configDir, "contexts.json")
    d,data, readErr := os.ReadFile(contextsPath)
    if readErr != nil {
            t.Fatalf("read %s after login: %v", contextsPath, readErr)
    }
    if !strings.Contains(string(data), server.URL) {
            t.Fatalf("contexts.json does not reference login server %s:\n%s", server.URL, data)
    }

serverState.Lock()
    polls := serverState.polls
    serverState.Unlock()

server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        
        switch {
        case r.Method == http.MethodPost && r.URL.Path == "/oauth/device/code":
        case r.Method == http.MethodPost && r.URL.Path == "/device_authorization":
            writeJSON(t, w, http.StatusOK, map[string]any{
                "device_code":               "device-expired",
                "user_code":                 "WXYZ-0000",
                
            }
        }

server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
            switch {
            case r.Method == http.MethodPost && r.URL.Path == "/oauth/device/code":
            case r.Method == http.MethodPost && r.URL.Path == "/device_authorization":
                writeJSON(t, w, http.StatusOK, map[string]any{
                    "device_code":               "device-denied",
                    "user_code":                 "QRST-9999",
                })
            }
    }

writeJSON(t, w, http.StatusOK, map[string]any{
                "access_token": "browser-token", "token_type": "Bearer", "expires_in": 3600, "scope": "cli offline_access",
                "access_token": fakeLoginJWT("http://" + r.Host), "token_type": "Bearer", "expires_in": 3600, "scope": "cli offline_access",
            })
            return
        }
    }

type loginProcess struct {
    stdout *bufio.Reader
    waitFn func() (string, error)
    configDir string
    waitFn    func() (string, error)
}

func runLoginProcess(t *testing.T, apiBaseURL string) *loginProcess {
    
    t.Helper()
    
    env := NewTestEnv(t)
    configDir := filepath.Join(env.RepoDir, ".entire-test-config")

// ENTIRE_AUTH_BASE_URL is retired (commands reject it when set at all);
    // --server is how a login targets the test server instead of the
    // production default.
    args = append(args, "--server", apiBaseURL)
    cmd := execx.NonInteractive(context.Background(), getTestBinary(), args...)
    cmd.Dir = env.RepoDir
    cmd.Env = append(testutil.GitIsolatedEnv(),
        "ENTIRE_TEST_GEMINI_PROJECT_DIR="+env.GeminiProjectDir,
        "ENTIRE_TEST_OPENCODE_PROJECT_DIR="+env.OpenCodeProjectDir,
        "ENTIRE_API_BASE_URL="+apiBaseURL,
        "ENTIRE_AUTH_BASE_URL="+apiBaseURL,
        "ENTIRE_TEST_AUTH_STORE_FILE="+filepath.Join(env.RepoDir, ".entire-test-auth-store.json"),
        "ENTIRE_CONFIG_DIR="+configDir,
        "ENTIRE_TOKEN_STORE=file",
        "ENTIRE_TOKEN_STORE_PATH="+filepath.Join(env.RepoDir, ".entire-test-tokens.json"),
        )
    
    reader := bufio.NewReader(stdoutPipe)

return &loginProcess{
    stdout: reader,
    configDir: configDir,
    waitFn: func() (string, error) {
            stdoutBytes, readErr := io.ReadAll(reader)
            waitErr := cmd.Wait()