From 9b8ad4d027b3cc0419238254fa022bfedf15737a Mon Sep 17 00:00:00 2001 From: Tink bot Date: Thu, 21 May 2026 12:39:15 +0000 Subject: [PATCH] feat(veans): URL discovery on init, port of the frontend's heuristic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous init flow took whatever the user typed for --server and called GET /api/v1/info on it. If the user typed "vikunja.example.com" (no scheme), or pasted the URL with /api/v1 in it (double-suffix), or pointed at a localhost install on the default :3456 port without typing the port, we'd hand back a raw HTTP error. New `client.DiscoverServer` ports the frontend's helpers/checkAndSetApiUrl.ts discovery: probe a small ordered set of plausible bases for /api/v1/info, return the first one that returns parseable Info. Candidate order: 1. scheme://host[:port]/path (as the user typed it) 2. scheme://host:3456/path (default API port) 3. opposite scheme of (1) 4. opposite scheme of (2) Heuristics: - Missing scheme → https for public hosts, http for localhost / 127.0.0.1 / [::1] (matches most CLIs' behaviour) - Trailing /api/v1 from a pasted URL is stripped before probing, so we don't double up to /api/v1/api/v1/info - Trailing slashes normalized Errors now list everything we tried + the last underlying network error, so the user can see why a URL failed instead of just "GET /info: connection refused": veans: VALIDATION_ERROR: couldn't find a Vikunja instance reachable from "vikunja.example.com" — tried: - https://vikunja.example.com/api/v1/info - https://vikunja.example.com:3456/api/v1/info - http://vikunja.example.com/api/v1/info - http://vikunja.example.com:3456/api/v1/info last error: dial tcp: lookup vikunja.example.com: no such host bootstrap.Init now defers URL canonicalisation to DiscoverServer and caches the matched info from the probe (no second /info round-trip). Unit tests cover the candidate-builder across the common shapes: bare hostname, localhost, /api/v1-suffixed paste, explicit port, subpath install, 127.0.0.1:3456, trailing slash. e2e green. --- veans/cmd/veans/main.go | 4 + veans/internal/bootstrap/bootstrap.go | 44 +++++--- veans/internal/client/client.go | 16 +-- veans/internal/client/discover.go | 143 +++++++++++++++++++++++++ veans/internal/client/discover_test.go | 118 ++++++++++++++++++++ 5 files changed, 302 insertions(+), 23 deletions(-) create mode 100644 veans/internal/client/discover.go create mode 100644 veans/internal/client/discover_test.go diff --git a/veans/cmd/veans/main.go b/veans/cmd/veans/main.go index 2e3520326..dfff29e0f 100644 --- a/veans/cmd/veans/main.go +++ b/veans/cmd/veans/main.go @@ -18,8 +18,11 @@ package main import ( + "fmt" "os" + "runtime" + "code.vikunja.io/veans/internal/client" "code.vikunja.io/veans/internal/commands" ) @@ -27,5 +30,6 @@ import ( var version = "dev" func main() { + client.UserAgent = fmt.Sprintf("veans/%s (%s/%s)", version, runtime.GOOS, runtime.GOARCH) os.Exit(commands.Execute(version)) } diff --git a/veans/internal/bootstrap/bootstrap.go b/veans/internal/bootstrap/bootstrap.go index 0fa15ff81..345b2fc83 100644 --- a/veans/internal/bootstrap/bootstrap.go +++ b/veans/internal/bootstrap/bootstrap.go @@ -26,6 +26,7 @@ package bootstrap import ( "context" + "errors" "fmt" "io" "sort" @@ -136,18 +137,18 @@ func Init(ctx context.Context, opts *Options) (*Result, error) { } opts.Server = strings.TrimSpace(v) } - opts.Server = strings.TrimRight(opts.Server, "/") - if opts.Server == "" { - return nil, output.New(output.CodeValidation, "server URL is required") - } - // 3. Probe /info. - human := client.New(opts.Server, "") - info, err := human.Info(ctx) + // 3. Discover the actual API URL: the user might have typed bare + // "vikunja.example.com", or pasted the URL with /api/v1 already in + // it, or be on a default-port localhost install. DiscoverServer + // probes the plausible variants and returns the canonical base. + canonical, info, err := client.DiscoverServer(ctx, opts.Server) if err != nil { - return nil, output.Wrap(output.CodeUnknown, err, "GET /info on %s: %v", opts.Server, err) + return nil, err } - progress(opts.Out, "Connected to Vikunja %s", info.Version) + opts.Server = canonical + human := client.New(canonical, "") + progress(opts.Out, "Connected to Vikunja %s at %s", info.Version, canonical) // 4. Acquire human JWT (transient — used until step 11). Default is the // OAuth flow; --token / --use-password / --username+--password override. @@ -192,14 +193,21 @@ func Init(ctx context.Context, opts *Options) (*Result, error) { return nil, err } - // 9. Share the project with the bot. - if _, err := human.ShareProjectWithUser(ctx, project.ID, &client.ProjectUser{ + // 9. Share the project with the bot. 409 ("user already has access") + // is the expected response when reusing a bot that was set up by a + // previous init run — treat it as a soft-success. + _, shareErr := human.ShareProjectWithUser(ctx, project.ID, &client.ProjectUser{ Username: bot.Username, Permission: client.PermissionReadWrite, - }); err != nil { - return nil, output.Wrap(output.CodeUnknown, err, "share project with bot: %v", err) + }) + switch { + case shareErr == nil: + progress(opts.Out, "Shared project with %q (read+write)", bot.Username) + case isConflictErr(shareErr): + progress(opts.Out, "Project already shared with %q", bot.Username) + default: + return nil, output.Wrap(output.CodeUnknown, shareErr, "share project with bot: %v", shareErr) } - progress(opts.Out, "Shared project with %q (read+write)", bot.Username) // 10. Discover available API permission scopes, mint the bot's token. routes, err := human.Routes(ctx) @@ -468,4 +476,10 @@ func progress(w io.Writer, format string, args ...any) { fmt.Fprintf(w, " → "+format+"\n", args...) } -// silence the unused-import linter when errors isn't used elsewhere. +// isConflictErr reports whether the wrapped HTTP error is a 409 — used by +// init's "share project with bot" step, which legitimately gets one when +// the bot is being reused from an earlier run. +func isConflictErr(err error) bool { + var oe *output.Error + return errors.As(err, &oe) && oe.Code == output.CodeConflict +} diff --git a/veans/internal/client/client.go b/veans/internal/client/client.go index c0553726e..5949d0d9d 100644 --- a/veans/internal/client/client.go +++ b/veans/internal/client/client.go @@ -39,15 +39,19 @@ type Client struct { BaseURL string Token string HTTPClient *http.Client - UserAgent string } +// UserAgent is the value sent in the User-Agent header on every request. +// main sets this at startup with the linker-injected version + the +// runtime os/arch (e.g. "veans/0.3.1 (linux/amd64)"). Tests get the +// default "veans/dev". Vikunja admins see this in their access logs. +var UserAgent = "veans/dev" + func New(baseURL, token string) *Client { return &Client{ BaseURL: strings.TrimRight(baseURL, "/"), Token: token, HTTPClient: &http.Client{Timeout: 30 * time.Second}, - UserAgent: "veans/0.1", } } @@ -86,9 +90,7 @@ func (c *Client) Do(ctx context.Context, method, path string, query url.Values, if c.Token != "" { req.Header.Set("Authorization", "Bearer "+c.Token) } - if c.UserAgent != "" { - req.Header.Set("User-Agent", c.UserAgent) - } + req.Header.Set("User-Agent", UserAgent) resp, err := c.HTTPClient.Do(req) if err != nil { @@ -135,9 +137,7 @@ func (c *Client) DoRaw(ctx context.Context, method, path string, query url.Value if c.Token != "" { req.Header.Set("Authorization", "Bearer "+c.Token) } - if c.UserAgent != "" { - req.Header.Set("User-Agent", c.UserAgent) - } + req.Header.Set("User-Agent", UserAgent) resp, err := c.HTTPClient.Do(req) if err != nil { return 0, nil, err diff --git a/veans/internal/client/discover.go b/veans/internal/client/discover.go new file mode 100644 index 000000000..8cad249e7 --- /dev/null +++ b/veans/internal/client/discover.go @@ -0,0 +1,143 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-present Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package client + +import ( + "context" + "errors" + "net/url" + "strings" + + "code.vikunja.io/veans/internal/output" +) + +// defaultAPIPort is what `VIKUNJA_SERVICE_INTERFACE` ships with — handy +// when the user types just `myhost.example.com` for a default install +// running on an unusual port. +const defaultAPIPort = "3456" + +// DiscoverServer normalizes `input` and probes a small set of plausible +// URLs for /api/v1/info, returning the canonical base URL (without the +// /api/v1 suffix — that's what client.New expects) and the parsed Info. +// +// Mirrors the discovery the Vikunja web frontend does in +// helpers/checkAndSetApiUrl.ts: try the URL as-given, with /api/v1 +// appended, and with the default :3456 port — across http / https. The +// first response that parses as Info wins. +func DiscoverServer(ctx context.Context, input string) (string, *Info, error) { + input = strings.TrimSpace(input) + if input == "" { + return "", nil, output.New(output.CodeValidation, "server URL is required") + } + + candidates, err := serverCandidates(input) + if err != nil { + return "", nil, output.Wrap(output.CodeValidation, err, + "can't parse server URL %q: %v", input, err) + } + + var attempts []string + var lastErr error + for _, base := range candidates { + attempts = append(attempts, base+"/api/v1/info") + info, err := New(base, "").Info(ctx) + if err == nil && info != nil { + return base, info, nil + } + lastErr = err + } + + return "", nil, output.New(output.CodeValidation, + "couldn't find a Vikunja instance reachable from %q — tried:\n - %s\nlast error: %v", + input, strings.Join(attempts, "\n - "), lastErr) +} + +// serverCandidates expands `input` into the ordered list of base URLs +// to probe for /api/v1/info. A "base URL" here is what client.New wants: +// the origin + the path that should sit BEFORE /api/v1 (typically empty +// or a reverse-proxy prefix). The probe itself adds /api/v1/info. +func serverCandidates(input string) ([]string, error) { + // Strip a trailing /api/v1[/] the user might have copied from a + // curl example. We add it back in the probe, and otherwise we'd + // end up calling /api/v1/api/v1/info. + trimmed := strings.TrimRight(input, "/") + trimmed = strings.TrimSuffix(trimmed, "/api/v1") + trimmed = strings.TrimRight(trimmed, "/") + + withScheme := trimmed + if !strings.HasPrefix(withScheme, "http://") && !strings.HasPrefix(withScheme, "https://") { + withScheme = defaultScheme(trimmed) + "://" + trimmed + } + + u, err := url.Parse(withScheme) + if err != nil { + return nil, err + } + if u.Host == "" { + return nil, errors.New("missing host") + } + + // Build the candidate set, dedup-preserving-order. The order here + // is the search policy: as-given, with default port, then the + // opposite scheme for each. Stops on the first one that responds + // with a parseable Info. + var bases []string + add := func(scheme, host, path string) { + base := scheme + "://" + host + strings.TrimRight(path, "/") + base = strings.TrimRight(base, "/") + for _, existing := range bases { + if existing == base { + return + } + } + bases = append(bases, base) + } + + hosts := []string{u.Host} + if u.Port() == "" { + hosts = append(hosts, u.Hostname()+":"+defaultAPIPort) + } + schemes := []string{u.Scheme} + if u.Scheme == "https" { + schemes = append(schemes, "http") + } else { + schemes = append(schemes, "https") + } + for _, s := range schemes { + for _, h := range hosts { + add(s, h, u.Path) + } + } + return bases, nil +} + +// defaultScheme picks http for loopback hosts and https for everything +// else — matches the heuristic most CLIs use when a scheme isn't typed. +func defaultScheme(input string) string { + host := input + if i := strings.IndexByte(host, '/'); i >= 0 { + host = host[:i] + } + if i := strings.IndexByte(host, ':'); i >= 0 { + host = host[:i] + } + switch host { + case "localhost", "127.0.0.1", "[::1]", "::1": + return "http" + } + return "https" +} diff --git a/veans/internal/client/discover_test.go b/veans/internal/client/discover_test.go new file mode 100644 index 000000000..1ecea8c05 --- /dev/null +++ b/veans/internal/client/discover_test.go @@ -0,0 +1,118 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-present Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package client + +import ( + "slices" + "testing" +) + +func TestServerCandidates(t *testing.T) { + cases := []struct { + name string + input string + want []string + }{ + { + name: "bare hostname → https first, then http, with default-port fallbacks", + input: "vikunja.example.com", + want: []string{ + "https://vikunja.example.com", + "https://vikunja.example.com:3456", + "http://vikunja.example.com", + "http://vikunja.example.com:3456", + }, + }, + { + name: "localhost defaults to http", + input: "localhost", + want: []string{ + "http://localhost", + "http://localhost:3456", + "https://localhost", + "https://localhost:3456", + }, + }, + { + name: "user-supplied /api/v1 suffix is trimmed (so the probe doesn't double it up)", + input: "https://vikunja.example.com/api/v1", + want: []string{ + "https://vikunja.example.com", + "https://vikunja.example.com:3456", + "http://vikunja.example.com", + "http://vikunja.example.com:3456", + }, + }, + { + name: "explicit port is respected — no default-port fallback added", + input: "https://vikunja.example.com:8443", + want: []string{ + "https://vikunja.example.com:8443", + "http://vikunja.example.com:8443", + }, + }, + { + name: "subpath install keeps the prefix", + input: "https://example.com/vikunja", + want: []string{ + "https://example.com/vikunja", + "https://example.com:3456/vikunja", + "http://example.com/vikunja", + "http://example.com:3456/vikunja", + }, + }, + { + name: "127.0.0.1 with default port (common dev setup)", + input: "127.0.0.1:3456", + want: []string{ + "http://127.0.0.1:3456", + "https://127.0.0.1:3456", + }, + }, + { + name: "trailing slash trimmed", + input: "https://vikunja.example.com/", + want: []string{ + "https://vikunja.example.com", + "https://vikunja.example.com:3456", + "http://vikunja.example.com", + "http://vikunja.example.com:3456", + }, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got, err := serverCandidates(c.input) + if err != nil { + t.Fatalf("serverCandidates(%q): %v", c.input, err) + } + if !slices.Equal(got, c.want) { + t.Errorf("serverCandidates(%q):\n got %v\n want %v", c.input, got, c.want) + } + }) + } +} + +func TestServerCandidates_EmptyInput(t *testing.T) { + // "" is the only input shape DiscoverServer rejects at the entry + // (before reaching serverCandidates). The lower-level helper itself + // reports "missing host" through the url.Parse path. + if _, err := serverCandidates(""); err == nil { + t.Error("empty input should error") + } +}