Error: %s
+Please return to the terminal.
`, html.EscapeString(result.Error)) + } else { + _, _ = fmt.Fprint(w, `You can close this window and return to the terminal.
+`) + } + + select { + case cs.ResultChan <- result: + default: + } +} + +func (cs *CallbackServer) Shutdown() { + if cs.server != nil { + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + _ = cs.server.Shutdown(ctx) + } +} + +func DefaultScopes() []string { + return append([]string{}, AvailableScopes...) +} + +func ExchangeCodeForToken(ctx context.Context, serverURL, code, verifier, redirectURI string) (*TokenResponse, error) { + data := url.Values{} + data.Set("code", code) + data.Set("code_verifier", verifier) + data.Set("redirect_uri", redirectURI) + + req, err := http.NewRequestWithContext(ctx, "POST", strings.TrimSuffix(serverURL, "/")+PkceTokenPath, strings.NewReader(data.Encode())) + if err != nil { + return nil, err + } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + + resp, err := http.DefaultClient.Do(req) + if err != nil { + return nil, fmt.Errorf("token request: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + body, err := io.ReadAll(io.LimitReader(resp.Body, maxResponseBody)) + if err != nil { + return nil, fmt.Errorf("read response: %w", err) + } + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("token exchange failed (status %d): %s", resp.StatusCode, body) + } + + var tokenResp TokenResponse + if err := json.Unmarshal(body, &tokenResp); err != nil { + return nil, fmt.Errorf("decode token response: %w", err) + } + return &tokenResp, nil +} diff --git a/internal/api/pkce_test.go b/internal/api/pkce_test.go new file mode 100644 index 0000000..db74080 --- /dev/null +++ b/internal/api/pkce_test.go @@ -0,0 +1,378 @@ +package api + +import ( + "context" + "encoding/base64" + "fmt" + "net" + "net/http" + "net/http/httptest" + "net/url" + "regexp" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGenerateCodeVerifier(T *testing.T) { + T.Parallel() + + T.Run("length is at least 43 characters", func(t *testing.T) { + t.Parallel() + + verifier, err := GenerateCodeVerifier() + require.NoError(t, err) + assert.GreaterOrEqual(t, len(verifier), 43, "RFC 7636 requires minimum 43 characters") + }) + + T.Run("contains only URL-safe characters", func(t *testing.T) { + t.Parallel() + + verifier, err := GenerateCodeVerifier() + require.NoError(t, err) + // base64url alphabet: A-Z, a-z, 0-9, -, _ + urlSafePattern := regexp.MustCompile(`^[A-Za-z0-9_-]+$`) + assert.True(t, urlSafePattern.MatchString(verifier), "verifier should only contain URL-safe base64 characters") + }) + + T.Run("no padding characters", func(t *testing.T) { + t.Parallel() + + verifier, err := GenerateCodeVerifier() + require.NoError(t, err) + assert.NotContains(t, verifier, "=", "verifier should not contain padding") + }) + + T.Run("generates unique values", func(t *testing.T) { + t.Parallel() + + v1, err := GenerateCodeVerifier() + require.NoError(t, err) + v2, err := GenerateCodeVerifier() + require.NoError(t, err) + assert.NotEqual(t, v1, v2, "verifiers should be unique") + }) +} + +func TestGenerateCodeChallenge(T *testing.T) { + T.Parallel() + + T.Run("produces valid base64url encoded SHA256 hash", func(t *testing.T) { + t.Parallel() + + verifier := "dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk" + challenge := GenerateCodeChallenge(verifier) + + // Should be base64url encoded (no padding, URL-safe chars) + urlSafePattern := regexp.MustCompile(`^[A-Za-z0-9_-]+$`) + assert.True(t, urlSafePattern.MatchString(challenge), "challenge should be base64url encoded") + }) + + T.Run("no padding characters", func(t *testing.T) { + t.Parallel() + + verifier, err := GenerateCodeVerifier() + require.NoError(t, err) + challenge := GenerateCodeChallenge(verifier) + assert.NotContains(t, challenge, "=", "challenge should not contain padding") + }) + + T.Run("same verifier produces same challenge", func(t *testing.T) { + t.Parallel() + + verifier := "test-verifier-12345" + c1 := GenerateCodeChallenge(verifier) + c2 := GenerateCodeChallenge(verifier) + assert.Equal(t, c1, c2, "same verifier should produce same challenge") + }) + + T.Run("different verifiers produce different challenges", func(t *testing.T) { + t.Parallel() + + c1 := GenerateCodeChallenge("verifier1") + c2 := GenerateCodeChallenge("verifier2") + assert.NotEqual(t, c1, c2, "different verifiers should produce different challenges") + }) + + T.Run("challenge can be decoded as valid base64url", func(t *testing.T) { + t.Parallel() + + verifier, err := GenerateCodeVerifier() + require.NoError(t, err) + challenge := GenerateCodeChallenge(verifier) + + decoded, err := base64.RawURLEncoding.DecodeString(challenge) + require.NoError(t, err, "challenge should be valid base64url") + assert.Len(t, decoded, 32, "SHA256 produces 32 bytes") + }) +} + +func TestGenerateState(T *testing.T) { + T.Parallel() + + T.Run("generates non-empty state", func(t *testing.T) { + t.Parallel() + + state, err := GenerateState() + require.NoError(t, err) + assert.NotEmpty(t, state) + }) + + T.Run("contains only URL-safe characters", func(t *testing.T) { + t.Parallel() + + state, err := GenerateState() + require.NoError(t, err) + urlSafePattern := regexp.MustCompile(`^[A-Za-z0-9_-]+$`) + assert.True(t, urlSafePattern.MatchString(state), "state should only contain URL-safe base64 characters") + }) + + T.Run("generates unique values", func(t *testing.T) { + t.Parallel() + + s1, err := GenerateState() + require.NoError(t, err) + s2, err := GenerateState() + require.NoError(t, err) + assert.NotEqual(t, s1, s2, "states should be unique") + }) +} + +func TestBuildAuthorizeURL(T *testing.T) { + T.Parallel() + + T.Run("includes all required parameters", func(t *testing.T) { + t.Parallel() + + authURL := BuildAuthorizeURL( + "https://teamcity.example.com", + "http://localhost:19000/callback", + "challenge123", + "state456", + []string{"RUN_BUILD", "VIEW_PROJECT"}, + ) + + parsed, err := url.Parse(authURL) + require.NoError(t, err) + + assert.Equal(t, "https", parsed.Scheme) + assert.Equal(t, "teamcity.example.com", parsed.Host) + assert.Equal(t, "/pkce/authorize.html", parsed.Path) + + query := parsed.Query() + assert.Equal(t, "code", query.Get("response_type")) + assert.Equal(t, "http://localhost:19000/callback", query.Get("redirect_uri")) + assert.Equal(t, "challenge123", query.Get("code_challenge")) + assert.Equal(t, "S256", query.Get("code_challenge_method")) + assert.Equal(t, "state456", query.Get("state")) + assert.Equal(t, "RUN_BUILD VIEW_PROJECT", query.Get("scope")) + }) + + T.Run("handles single scope", func(t *testing.T) { + t.Parallel() + + authURL := BuildAuthorizeURL( + "https://teamcity.example.com", + "http://localhost:19000/callback", + "challenge", + "state", + []string{"RUN_BUILD"}, + ) + + parsed, err := url.Parse(authURL) + require.NoError(t, err) + assert.Equal(t, "RUN_BUILD", parsed.Query().Get("scope")) + }) + + T.Run("strips trailing slash from server URL", func(t *testing.T) { + t.Parallel() + + authURL := BuildAuthorizeURL( + "https://teamcity.example.com/", + "http://localhost:19000/callback", + "challenge", + "state", + []string{"RUN_BUILD"}, + ) + + assert.True(t, strings.HasPrefix(authURL, "https://teamcity.example.com/pkce/")) + assert.NotContains(t, authURL, "//pkce") + }) +} + +func TestFindAvailableListener(T *testing.T) { + T.Parallel() + + T.Run("returns port in valid range", func(t *testing.T) { + t.Parallel() + + listener, port, err := FindAvailableListener() + require.NoError(t, err) + defer listener.Close() + + assert.GreaterOrEqual(t, port, CallbackPortMin) + assert.LessOrEqual(t, port, CallbackPortMax) + }) + + T.Run("returned listener is usable", func(t *testing.T) { + t.Parallel() + + listener, port, err := FindAvailableListener() + require.NoError(t, err) + defer listener.Close() + + // Listener should be bound to the reported port + addr := listener.Addr().(*net.TCPAddr) + assert.Equal(t, port, addr.Port) + }) +} + +func TestIsPkceEnabled(T *testing.T) { + T.Parallel() + + T.Run("returns true when server responds 200", func(t *testing.T) { + t.Parallel() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "POST", r.Method) + assert.Equal(t, "/pkce/is_enabled.html", r.URL.Path) + w.WriteHeader(http.StatusOK) + })) + t.Cleanup(server.Close) + + enabled, err := IsPkceEnabled(context.Background(), server.URL) + assert.NoError(t, err) + assert.True(t, enabled) + }) + + T.Run("returns false when server responds 404", func(t *testing.T) { + t.Parallel() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + })) + t.Cleanup(server.Close) + + enabled, err := IsPkceEnabled(context.Background(), server.URL) + assert.NoError(t, err) + assert.False(t, enabled) + }) + + T.Run("returns error on network failure", func(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + defer cancel() + enabled, err := IsPkceEnabled(ctx, "http://localhost:1") + assert.Error(t, err) + assert.False(t, enabled) + }) +} + +func TestCallbackServer(T *testing.T) { + T.Run("receives authorization code from callback", func(t *testing.T) { + listener, port, err := FindAvailableListener() + require.NoError(t, err) + + server := NewCallbackServer(listener, port) + server.Start() + defer server.Shutdown() + + go func() { + time.Sleep(50 * time.Millisecond) + resp, _ := http.Get(fmt.Sprintf("http://localhost:%d/callback?code=testcode123&state=teststate456", port)) + if resp != nil { + resp.Body.Close() + } + }() + + select { + case result := <-server.ResultChan: + assert.Equal(t, "testcode123", result.Code) + assert.Equal(t, "teststate456", result.State) + assert.Empty(t, result.Error) + case <-time.After(2 * time.Second): + t.Fatal("timeout waiting for callback") + } + }) + + T.Run("handles error in callback", func(t *testing.T) { + listener, port, err := FindAvailableListener() + require.NoError(t, err) + + server := NewCallbackServer(listener, port) + server.Start() + defer server.Shutdown() + + go func() { + time.Sleep(50 * time.Millisecond) + resp, _ := http.Get(fmt.Sprintf("http://localhost:%d/callback?error=access_denied", port)) + if resp != nil { + resp.Body.Close() + } + }() + + select { + case result := <-server.ResultChan: + assert.Empty(t, result.Code) + assert.Equal(t, "access_denied", result.Error) + case <-time.After(2 * time.Second): + t.Fatal("timeout waiting for callback") + } + }) +} + +func TestExchangeCodeForToken(T *testing.T) { + T.Parallel() + + T.Run("exchanges code for token successfully", func(t *testing.T) { + t.Parallel() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "POST", r.Method) + assert.Equal(t, "/pkce/token.html", r.URL.Path) + require.NoError(t, r.ParseForm()) + assert.Equal(t, "testcode", r.Form.Get("code")) + assert.Equal(t, "testverifier", r.Form.Get("code_verifier")) + assert.Equal(t, "http://localhost:19000/callback", r.Form.Get("redirect_uri")) + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"access_token":"token123","token_type":"Bearer","valid_until":"2026-03-03T11:25:51.572Z"}`)) + })) + t.Cleanup(server.Close) + + token, err := ExchangeCodeForToken(context.Background(), server.URL, "testcode", "testverifier", "http://localhost:19000/callback") + require.NoError(t, err) + assert.Equal(t, "token123", token.AccessToken) + assert.Equal(t, "Bearer", token.TokenType) + assert.Equal(t, "2026-03-03T11:25:51.572Z", token.ValidUntil) + }) + + T.Run("returns error on invalid code", func(t *testing.T) { + t.Parallel() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusForbidden) + w.Write([]byte("Invalid authorization code")) + })) + t.Cleanup(server.Close) + + _, err := ExchangeCodeForToken(context.Background(), server.URL, "invalidcode", "verifier", "http://localhost:19000/callback") + assert.Error(t, err) + }) +} + +func TestDefaultScopes(T *testing.T) { + T.Parallel() + + T.Run("returns copy of available scopes", func(t *testing.T) { + t.Parallel() + + scopes := DefaultScopes() + assert.Equal(t, AvailableScopes, scopes) + scopes[0] = "MODIFIED" + assert.NotEqual(t, AvailableScopes[0], "MODIFIED") + }) +} diff --git a/internal/api/testenv_test.go b/internal/api/testenv_test.go index 41cc517..9302716 100644 --- a/internal/api/testenv_test.go +++ b/internal/api/testenv_test.go @@ -141,7 +141,7 @@ func startContainers() (*testEnv, error) { env.network.Name: {"teamcity-server"}, }, Env: map[string]string{ - "TEAMCITY_SERVER_OPTS": "-Dteamcity.installation.completed=true -Dteamcity.startup.maintenance=false -Dteamcity.licenseAgreement.accepted=true", + "TEAMCITY_SERVER_OPTS": "-Dteamcity.installation.completed=true -Dteamcity.startup.maintenance=false -Dteamcity.licenseAgreement.accepted=true -Dteamcity.internal.server.oauth.pkce.enable=true", }, WaitingFor: wait.ForHTTP("/app/rest/server/version"). WithPort("8111/tcp"). diff --git a/internal/cmd/auth.go b/internal/cmd/auth.go index c58892a..fe1097b 100644 --- a/internal/cmd/auth.go +++ b/internal/cmd/auth.go @@ -1,8 +1,11 @@ package cmd import ( + "context" + "crypto/subtle" "fmt" "strings" + "time" "github.com/AlecAivazis/survey/v2" "github.com/JetBrains/teamcity-cli/internal/api" @@ -13,6 +16,9 @@ import ( "github.com/spf13/cobra" ) +// authCodeLifetime is the maximum time to wait for the OAuth callback +const authCodeLifetime = 5 * time.Minute + func newAuthCmd() *cobra.Command { cmd := &cobra.Command{ Use: "auth", @@ -32,6 +38,8 @@ func newAuthCmd() *cobra.Command { func newAuthLoginCmd() *cobra.Command { var serverURL string var token string + var noBrowser bool + var scopes []string cmd := &cobra.Command{ Use: "login", @@ -40,8 +48,9 @@ func newAuthLoginCmd() *cobra.Command { This will: 1. Prompt for your TeamCity server URL -2. Open your browser to generate an access token -3. Validate and store the token securely +2. Automatically authenticate via browser (if PKCE is enabled) +3. Or open your browser to generate an access token manually +4. Validate and store the token securely For CI/CD, use environment variables instead: export TEAMCITY_URL="https://teamcity.example.com" @@ -50,29 +59,47 @@ For CI/CD, use environment variables instead: When running inside a TeamCity build, authentication is automatic using build-level credentials from the build properties file.`, RunE: func(cmd *cobra.Command, args []string) error { - return runAuthLogin(serverURL, token) + return runAuthLogin(serverURL, token, noBrowser, scopes) }, } cmd.Flags().StringVarP(&serverURL, "server", "s", "", "TeamCity server URL") cmd.Flags().StringVarP(&token, "token", "t", "", "Access token") + cmd.Flags().BoolVar(&noBrowser, "no-browser", false, "Skip browser-based auth, use manual token entry") + cmd.Flags().StringSliceVar(&scopes, "scopes", api.DefaultScopes(), "Permissions for the token (PKCE only)") return cmd } -func runAuthLogin(serverURL, token string) error { +func runAuthLogin(serverURL, token string, noBrowser bool, scopes []string) error { isInteractive := !NoInput && output.IsStdinTerminal() if serverURL == "" { + // Try to detect server from DSL (pom.xml) + detectedServer := config.DetectServerFromDSL() + if !isInteractive { - return tcerrors.RequiredFlag("server") - } - prompt := &survey.Input{ - Message: "TeamCity server URL:", - Help: "e.g., https://teamcity.example.com", - } - if err := survey.AskOne(prompt, &serverURL, survey.WithValidator(survey.Required)); err != nil { - return err + if detectedServer != "" { + serverURL = detectedServer + } else { + return tcerrors.RequiredFlag("server") + } + } else { + // Interactive mode: let user confirm or change detected server + prompt := &survey.Input{ + Message: "TeamCity server URL:", + Help: "e.g., https://teamcity.example.com", + } + + if detectedServer != "" { + prompt.Default = detectedServer + dslDir := config.DetectTeamCityDir() + fmt.Printf("%s Detected server from %s/pom.xml\n", output.Green("✓"), dslDir) + } + + if err := survey.AskOne(prompt, &serverURL, survey.WithValidator(survey.Required)); err != nil { + return err + } } } @@ -81,6 +108,27 @@ func runAuthLogin(serverURL, token string) error { serverURL = "https://" + serverURL } + // Try PKCE authentication first (if available and allowed) + var tokenValidUntil string + pkceChecked := false + if token == "" && !noBrowser && isInteractive { + pkceChecked = true + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + enabled, _ := api.IsPkceEnabled(ctx, serverURL) + cancel() + if enabled { + output.Info("Secure browser login enabled on this server") + if tokenResp, err := runPkceLogin(serverURL, scopes); err != nil { + output.Warn("Browser auth failed: %v", err) + output.Info("Falling back to manual token entry...") + } else { + token = tokenResp.AccessToken + tokenValidUntil = tokenResp.ValidUntil + } + } + } + + // Fall back to manual token entry if token == "" { if !isInteractive { return tcerrors.RequiredFlag("token") @@ -89,6 +137,10 @@ func runAuthLogin(serverURL, token string) error { tokenURL := fmt.Sprintf("%s/profile.html?item=accessTokens", serverURL) fmt.Println() + if pkceChecked { + fmt.Println(output.Faint("Tip: Server admins can enable secure browser login for easier authentication")) + fmt.Println() + } fmt.Println(output.Yellow("!"), "To authenticate, you need an access token.") fmt.Printf(" Generate one at: %s\n", tokenURL) fmt.Println() @@ -135,11 +187,65 @@ func runAuthLogin(serverURL, token string) error { } output.Success("Logged in as %s", output.Cyan(user.Name)) + if tokenValidUntil != "" { + if expiry, err := time.Parse(time.RFC3339, tokenValidUntil); err == nil { + output.Info("Token expires: %s", output.Yellow(expiry.Local().Format("Jan 2, 2006"))) + } + output.Info(output.Faint("Note: Some scopes may be restricted by server configuration.")) + } output.Info("\nConfiguration saved to %s", config.ConfigPath()) return nil } +func runPkceLogin(serverURL string, scopes []string) (*api.TokenResponse, error) { + verifier, err := api.GenerateCodeVerifier() + if err != nil { + return nil, fmt.Errorf("generate code verifier: %w", err) + } + state, err := api.GenerateState() + if err != nil { + return nil, fmt.Errorf("generate state: %w", err) + } + + listener, port, err := api.FindAvailableListener() + if err != nil { + return nil, fmt.Errorf("find available port: %w", err) + } + + callbackServer := api.NewCallbackServer(listener, port) + callbackServer.Start() + defer callbackServer.Shutdown() + + redirectURI := fmt.Sprintf("http://localhost:%d%s", port, api.DefaultCallbackPath) + authURL := api.BuildAuthorizeURL(serverURL, redirectURI, api.GenerateCodeChallenge(verifier), state, scopes) + + output.Info("Opening browser for authentication...") + fmt.Printf(" %s Approve access in TeamCity\n", output.Yellow("→")) + + if err := browser.OpenURL(authURL); err != nil { + return nil, fmt.Errorf("open browser: %w", err) + } + + select { + case result := <-callbackServer.ResultChan: + if result.Error != "" { + return nil, fmt.Errorf("authorization denied: %s", result.Error) + } + if subtle.ConstantTimeCompare([]byte(result.State), []byte(state)) != 1 { + return nil, fmt.Errorf("state mismatch: possible CSRF attack") + } + fmt.Println() + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + return api.ExchangeCodeForToken(ctx, serverURL, result.Code, verifier, redirectURI) + + case <-time.After(authCodeLifetime): + return nil, fmt.Errorf("timeout waiting for callback (exceeded %v)", authCodeLifetime) + } +} + func newAuthLogoutCmd() *cobra.Command { cmd := &cobra.Command{ Use: "logout",