diff --git a/.env.example b/.env.example index 01fbcdc..ad45d1d 100644 --- a/.env.example +++ b/.env.example @@ -2,3 +2,10 @@ CONFLUENCE_URL=https://your-org.atlassian.net CONFLUENCE_USERNAME=you@example.com CONFLUENCE_TOKEN=your-api-token + +# Optional. Set this only for a *scoped* API token, such as one issued to a +# service account: those must go through Atlassian's api.atlassian.com gateway +# rather than your site domain. Leave it out for a normal personal token. +# Find yours (it isn't a secret): +# curl -s https://your-org.atlassian.net/_edge/tenant_info +#CONFLUENCE_CLOUD_ID= diff --git a/CLAUDE.md b/CLAUDE.md index 8ee7b87..dbbcb4b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -19,13 +19,18 @@ Run `make test && make lint && make vet` before considering work done. ## Configuration -The CLI needs a base URL, a username, and an API token. Each resolves with the precedence **flag > environment variable > `.env` file**: +The CLI needs a site URL, a username, and an API token, plus an optional cloud ID. Each resolves with the precedence **flag > environment variable > `.env` file**: | Setting | Flag | Env / `.env` | |---|---|---| -| base URL | `--url` | `CONFLUENCE_URL` | +| site URL | `--url` | `CONFLUENCE_URL` | | username | `--username` | `CONFLUENCE_USERNAME` | | API token | *(none)* | `CONFLUENCE_TOKEN` | +| cloud ID (optional) | `--cloud-id` | `CONFLUENCE_CLOUD_ID` | + +A cloud ID routes requests through `https://api.atlassian.com/ex/confluence/{cloudId}` instead of the site domain, which is mandatory for a **scoped** API token (what an Atlassian service account gets — a scoped token 401s against the site domain). Basic auth is unchanged; only the base URL moves. Without a cloud ID, behavior is identical to before it existed, which is what an unscoped personal token and any Data Center site need. The cloud ID is not a secret (`https://SITE/_edge/tenant_info` returns it unauthenticated), which is why it may be a flag while the token may not. + +Scopes markfluence needs (it never deletes): `read:confluence-content.all`, `write:confluence-content`, `read:confluence-space.summary`, `read:confluence-props`, `write:confluence-props`, `write:confluence-file`, `read:confluence-user`. markfluence reads a `.env` from the working directory itself (a minimal built-in parser — no shell expansion), or an explicit path via the persistent `--env-file` flag (a missing explicit path is an error; a missing default `./.env` is not); `.env.example` is the template. The API token is deliberately never a command-line flag. `internal/client.Resolve` is the single place this is read and validated. @@ -43,7 +48,7 @@ Module `github.com/mozilla/markfluence` (`go 1.25`). `main.go` is a shim to `cmd - `cmd/root.go` — the cobra root: `--url`/`--username`/`--debug`/`--no-color` persistent flags, version from `internal/buildinfo`, and registration of the four subcommands. `Execute()` prints cobra-generated errors (bad args/flags) but not `ui.ErrSilent`, which marks a failure a command already reported. - `cmd/{update,create,fix,info}/` — one package per command (each exports `Cmd`), orchestrating the `internal` packages and `internal/ui` output. `create` is two-phase and transactional (validate all, then create parents-first in topological order); `fix` is read-only on the server. -- `internal/client` — `ConfluenceClient` over `net/http` with basic auth. Pages are Confluence **v2**; attachment writes and the user lookup are **v1** (`/wiki/rest/api/...`). Typed `HTTPError`, per-attempt context timeouts, centralized retry/backoff in `send` (429 for any method honoring `Retry-After`, plus 502/503/504 and network errors for idempotent methods only; exponential backoff capped), `SetContentProperty` retry-once on top (recovers a lost create-POST response), `SyncAttachments` (SHA-256-in-comment skip/update), `_links.next` pagination. `config.go` holds `Resolve` and the `.env` reader. +- `internal/client` — `ConfluenceClient` over `net/http` with basic auth. Built from a `Config` (site URL, cloud ID, username, token) via `New`; it carries **two bases**: `BaseURL()` is where requests go (the gateway when a cloud ID is set) and `SiteURL()` is always the site. Anything a reader sees uses `SiteURL()` — printed page URLs and, critically, the `baseURL` handed to `convert.MdToConfluence`, since rewritten links are published *into* the page. Pages are Confluence **v2**; attachment writes and the user lookup are **v1** (`/wiki/rest/api/...`). Typed `HTTPError`, per-attempt context timeouts, centralized retry/backoff in `send` (429 for any method honoring `Retry-After`, plus 502/503/504 and network errors for idempotent methods only; exponential backoff capped), `SetContentProperty` retry-once on top (recovers a lost create-POST response), `SyncAttachments` (SHA-256-in-comment skip/update), `_links.next` pagination. `config.go` holds `Resolve` and the `.env` reader. - `internal/convert` — the converter (the crux). `MdToConfluence(md *frontmatter.MarkdownFile, baseURL, spaceKey, version string) (*ConfluencePage, error)`. It parses with goldmark (GFM) and renders through a custom `storageRenderer` registered at priority 100 (below the default HTML=1000 and table=500 renderers) that emits Confluence storage format. `shield.go` renames raw `ac:`/`ri:` tags to colon-free sentinels around the goldmark step so pasted storage passes through; `callouts.go` is an AST transformer + blockquote renderer for GitHub alerts; `images.go`, `links.go` (sibling-file scans, GitHub/Confluence slugs, doc-link + anchor rewriting), `tables.go` (the `` tag only, stamped with Confluence's `data-layout="align-start"` so tables auto-size to their content and left-align; rows and cells still fall through to the GFM renderer), and `renderer.go` (code macros, text soft-break→space, images, links) do the rest. The `` and `` token substitutions happen **inside** `MdToConfluence`. - `internal/frontmatter` — flat YAML frontmatter parse/quote/`UpdateField`, and the `MarkdownFile` type (`Parse`/`ParseFile`, exported `Filename`/`Content`/`Frontmatter`/`Body`, and `Title`/`PageID`/`Space`/`Parent` accessors that normalize missing/blank/`"null"`). - `internal/pagewidth` — the `page_width` `Width` enum (`narrow`/`wide`/`max`, default `max`), `Declared`, the vocab↔content-property maps, `WidthFromProperties`, and `Apply`/`Read` against the client. diff --git a/README.md b/README.md index 777da17..ed451ac 100644 --- a/README.md +++ b/README.md @@ -23,14 +23,15 @@ TBD — published to a tap on the first release. ## Configure -markfluence needs a base URL, a username, and an API token. Each is resolved +markfluence needs a site URL, a username, and an API token. Each is resolved with the precedence **flag > environment variable > `.env` file**: | Setting | Flag | Environment / `.env` | | --- | --- | --- | -| Base URL | `--url` | `CONFLUENCE_URL` | +| Site URL | `--url` | `CONFLUENCE_URL` | | Username | `--username` | `CONFLUENCE_USERNAME` | | API token | *(none — never a flag)* | `CONFLUENCE_TOKEN` | +| Cloud ID *(optional)* | `--cloud-id` | `CONFLUENCE_CLOUD_ID` | markfluence reads a `.env` file from the current directory automatically (no need to `source` it), or from an explicit path via `--env-file PATH` (a persistent flag @@ -48,6 +49,40 @@ from the environment or `.env`. (Optional): `alias mf=markfluence` +### Scoped tokens and service accounts + +For a normal personal API token, you can leave `CONFLUENCE_CLOUD_ID` unset. + +For a **scoped** API token for an Atlassian [service account][svcacct], you +need to set `CONFLUENCE_CLOUD_ID`. You would use this to publish from CI as a +service account rather than as a person. Scoped tokens are rejected with a +**401** against your site domain; they must go through Atlassian's +`api.atlassian.com` gateway, and the cloud ID is what addresses your site +there. `CONFLUENCE_URL` still holds the site URL: markfluence needs it to write +correct links into the pages it publishes. + +Find your cloud ID — it is **not** a secret: + +```console +$ curl -s https://your-org.atlassian.net/_edge/tenant_info +{"cloudId":"d8febd08-5555-5555-5555-db37c2369ce5"} +``` + +The scopes markfluence needs: + +| Used for | Classic scope | +| --- | --- | +| Reading, creating, and updating pages | `read:confluence-content.all`, `write:confluence-content` | +| Resolving space keys | `read:confluence-space.summary` | +| Page width (content properties) | `read:confluence-props`, `write:confluence-props` | +| Image attachments | `write:confluence-file` | +| Author names in `info` | `read:confluence-user` | + +Currently, markfluence doesn't support deleting anything, so it doesn't need +delete scopes. This might change in the future. + +[svcacct]: https://support.atlassian.com/user-management/docs/understand-service-accounts/ + ## Usage ```sh @@ -301,6 +336,13 @@ markfluence reads them straight from the environment — no `.env` in CI. - `CONFLUENCE_URL` - `CONFLUENCE_USERNAME` +Prefer a [service account][svcacct] over a personal token here, so published pages +aren't authored by an individual and publishing doesn't break when that person +rotates their token or moves on. That means a **scoped** token, which also needs +`CONFLUENCE_CLOUD_ID` (see [Scoped tokens and service +accounts](#scoped-tokens-and-service-accounts)). The cloud ID is not sensitive, so +make it a repository **variable** rather than a secret. + [secrets]: https://docs.github.com/en/actions/security-guides/using-secrets-in-github-actions ### Workflow @@ -338,6 +380,9 @@ jobs: CONFLUENCE_URL: ${{ secrets.CONFLUENCE_URL }} CONFLUENCE_USERNAME: ${{ secrets.CONFLUENCE_USERNAME }} CONFLUENCE_TOKEN: ${{ secrets.CONFLUENCE_TOKEN }} + # A variable, not a secret: the cloud ID is public. Omit it if you're + # using an unscoped personal token. + CONFLUENCE_CLOUD_ID: ${{ vars.CONFLUENCE_CLOUD_ID }} run: markfluence update --page-id=12345 --force docs/some_doc.md ``` diff --git a/_plans/017_gateway-scoped-tokens.md b/_plans/017_gateway-scoped-tokens.md new file mode 100644 index 0000000..cfe50a3 --- /dev/null +++ b/_plans/017_gateway-scoped-tokens.md @@ -0,0 +1,205 @@ +# Plan: gateway base URL for scoped service-account API tokens + +Let markfluence authenticate as an Atlassian **service account** using a **scoped +API token**, so publishing from CI runs as the service account rather than as a +person. + +The blocker is not authentication. A scoped token still uses basic auth with +`email:token` — Atlassian's own [401 KB][401kb] shows exactly that. What breaks is +the **URL**: scoped tokens are rejected (401) against the site domain and must go +through the platform API gateway: + +``` +https://api.atlassian.com/ex/confluence/{cloudId}/wiki/api/v2/pages +``` + +The path suffix is unchanged, so every existing `baseURL + "/wiki/..."` call site +works verbatim once the base moves. The work is therefore: **split the one base URL +markfluence has today into a request base and a site base**, because the site URL is +still needed for content markfluence *writes* (link rewriting) and for URLs it +prints. + +Confirmation this is the right shape: [`pchuri/confluence-cli`][ccli] supports +scoped tokens with no dedicated credential type at all — you just point its +`--domain`/`--api-path` at the gateway and keep basic auth. + +[401kb]: https://support.atlassian.com/atlassian-cloud/kb/401-unauthorized-error-when-service-account-accesses-jira-or-confluence-api/ +[ccli]: https://github.com/pchuri/confluence-cli + +## Out of scope (deliberately) + +- **OAuth 2.0.** Atlassian's service-account OAuth is `client_credentials` (2LO), so + it *is* CI-usable — but the `client_secret` that mints the 60-minute token is + itself long-lived, so it shortens no secret we store in GitHub. It adds a token + endpoint and expiry handling and requires this same gateway split anyway. No + benefit here. +- **Bearer auth.** The bearer value *is* the scoped API token — same secret, different + header. It grants no capability basic auth lacks. Its only real payoff is not + needing the service account's email address. Worth doing later as a ~5-line + `switch` in `send`, but it is not part of what unblocks the service account, so it + lands separately once this is proven against the real token. +- **Cloud-ID auto-discovery.** `/_edge/tenant_info` returns the cloud ID but is not a + supported Atlassian API. Keep the cloud ID explicit config. + +## Decisions locked + +### `--url` stays the site URL; the gateway is derived from a new cloud ID + +The gateway URL cannot simply *replace* `CONFLUENCE_URL`, because the site URL is +still needed for two things — so it stays, and the only new fact is the cloud ID: + +| Setting | Flag | Env / `.env` | +|---|---|---| +| site URL | `--url` | `CONFLUENCE_URL` | +| username | `--username` | `CONFLUENCE_USERNAME` | +| API token | *(none — never a flag)* | `CONFLUENCE_TOKEN` | +| **cloud ID** | **`--cloud-id`** | **`CONFLUENCE_CLOUD_ID`** | + +- With a cloud ID set → requests go to `https://api.atlassian.com/ex/confluence/{cloudId}`. +- With it empty → **behavior is byte-identical to today**. This is the back-compat + guarantee: existing personal-token users and every current test are unaffected. +- **The cloud ID is not a secret.** `https://.atlassian.net/_edge/tenant_info` + returns it unauthenticated (the supported route is + `GET https://api.atlassian.com/oauth/token/accessible-resources`, whose `id` is the + cloud ID). So in CI it belongs in a repo **variable**, not a secret — and it's why it + can be a `--cloud-id` flag while the token deliberately cannot. +- Only **Cloud** sites have a cloud ID, and it identifies the *site*, not the product — + one ID serves Confluence and Jira on the same site. Data Center/Server has neither a + cloud ID nor the gateway, which is a second reason the setting must stay optional. + +Rejected: confluence-cli's flat `--api-path`. It works for them because they are +v1-only; markfluence mixes v1 (`/wiki/rest/api/content/.../child/attachment`, +`/wiki/rest/api/user`) and v2 (`/wiki/api/v2/pages`, `.../properties`), and one path +string cannot express both. + +### The client carries two bases + +`ConfluenceClient` (`internal/client/client.go:53`) gains `siteURL` alongside +`baseURL`, where **`baseURL` becomes strictly the request base**: + +- `baseURL` = gateway when a cloud ID is set, else the site URL. +- `siteURL` = always the site URL. +- New accessor `SiteURL()` beside the existing `BaseURL()`. + +`New` moves from positional args to a `Config` struct (`SiteURL`, `CloudID`, +`Username`, `Token`). Five string positional params — two of them URLs, one a secret +— transpose too easily to leave unnamed, and the struct extends cleanly when bearer +lands. Contained change: `New` has exactly **one** production caller (`Resolve`) and +two test helpers (`internal/client/client_test.go:47,140`). + +`Resolve` likewise moves to a named-field `Options` struct (`URL`, `Username`, +`CloudID`, `EnvFile`) rather than growing to a fourth positional string. Its five +call sites each pull persistent flags by name from cobra (`cmd/update/update.go:67`, +`cmd/create/create.go:94`, `cmd/fix/fix.go:43`, `cmd/info/info.go:44`, +`cmd/read/read.go:62`) and gain a `--cloud-id` pull. + +`send` (`internal/client/client.go:207`) is **untouched** — basic auth is correct for +scoped tokens. + +### Which call sites switch to `SiteURL()` + +**Content-facing — the correctness-critical one.** `MdToConfluence` receives +`c.SiteURL()` at `cmd/update/update.go:159` and `cmd/create/create.go:386`. Nothing +inside `internal/convert` changes; its `baseURL` param just receives the right value. +Getting this wrong writes `api.atlassian.com` links into **published pages** — a +silent, persistent defect that outlives the run, unlike a wrong printed URL. + +**Human-facing.** These already prefer the API's `page.Links.Base` and only fall back +to `BaseURL()`, so they largely self-heal; the fallbacks still move to `SiteURL()`: +`cmd/update/update.go:284,288`, `cmd/create/create.go:485,489`, +`cmd/info/info.go:144,146`, `cmd/fix/fix.go:177`. + +### `_links.next` must not double the gateway prefix + +`ListContentProperties` builds the next page as `c.baseURL + out.Links.Next` +(`internal/client/client.go:659`) — the only pagination site in the client. `next` is +a site-relative absolute path (`/wiki/api/v2/...`), so concatenation is what +*preserves* the `/ex/confluence/{cloudId}` prefix and is correct today. + +The risk is only if the gateway echoes `next` already carrying that prefix, which +would double it. Rather than guess the live behavior, add a small +`resolveNext(base, next)` helper handling three cases: + +1. `next` is absolute (has a scheme) → use as-is. +2. `next` already starts with `base`'s path prefix → prepend scheme+host only. +3. otherwise → concatenate onto `base` (today's behavior). + +Note `url.ResolveReference` is **wrong** here: an absolute-path reference replaces the +whole path, silently dropping `/ex/confluence/{cloudId}`. + +## Verified against the live API + +Smoke-tested with `info` through the gateway using an existing *personal* token +(gateway routing turns out to accept one, which made this testable before the service +account exists): + +- **`_links.base` returns the site URL under the gateway** — confirmed + `https://mozilla-hub.atlassian.net/wiki` from a v2 page fetch via + `api.atlassian.com`. So **no inversion is needed**: the `Links.Base`-preferring + logic in `cmd/info/info.go`, `cmd/update/update.go`, and `cmd/create/create.go` + stays as-is, and printed URLs are correct in both modes. +- **v1 endpoints work through the gateway** — `info` resolved author display names, + which is the v1 `/wiki/rest/api/user` call. + +Still open, and **only answerable with the scoped token**: + +- **The v1 attachment upload.** Atlassian documents classic scopes for v1 vs granular + for v2 while warning against mixing sets, so a scope gap would surface there, and it + is load-bearing for any page with images. A personal token is unscoped, so testing + this now would prove nothing about scopes — it has to wait for the real credential. + First real run should be an `update --dry-run` on a page with an image, then a live + one. + +### Scopes to request for the service account + +markfluence never deletes, so no delete scopes. Request the **classic** set (Atlassian's +recommendation): + +| Calls | Classic | Granular | +|---|---|---| +| GET/POST/PUT `/api/v2/pages` | `read:confluence-content.all`, `write:confluence-content` | `read:page:confluence`, `write:page:confluence` | +| GET `/api/v2/spaces` | `read:confluence-space.summary` | `read:space:confluence` | +| `/api/v2/pages/{id}/properties` (page width) | `read:confluence-props`, `write:confluence-props` | `read:content.property:confluence`, `write:content.property:confluence` | +| `/rest/api/content/{id}/child/attachment` (v1, images) | `write:confluence-file` | `read:attachment:confluence`, `write:attachment:confluence` | +| GET `/rest/api/user` (v1) | `read:confluence-user` | `read:user:confluence` | + +## Schema (`schema/json-output/v1.json`) — no change + +`url` fields already exist (lines 151, 198, 235); only their *values* could change, not +the document shape. `TestSchemaConformance` should stay green — confirm rather than +assume. + +## Testing + +- `internal/client/config_test.go`: cloud-ID resolution through the existing + flag > env > `.env` precedence; a cloud ID containing a slash or a full URL is + rejected with a useful message (guards against a pasted + `https://api.atlassian.com/ex/confluence/...`, which would otherwise 404 opaquely). +- `internal/client/client_test.go`: `New` with a cloud ID targets the gateway path + while `SiteURL()` stays the site; with no cloud ID both equal the site URL + (back-compat). +- `resolveNext`: unit-test all three branches, including the prefix-already-present + case that would otherwise double. +- **A `cmd`-level test that the converter receives the site URL in gateway mode.** + This is the one regression that silently publishes broken links, so it gets a + direct test rather than relying on client-level coverage. + +## Docs + +- `README.md`: config table (31–33), `.env` block (41–43), env-var list (~300), and a + service-account example in the GitHub Actions section (~338) — including that the + gateway needs `CONFLUENCE_CLOUD_ID` while `CONFLUENCE_URL` stays the site URL, + since that pairing is the non-obvious part. In that example `CONFLUENCE_CLOUD_ID` + must be a `vars.` entry, **not** grouped with the `secrets.` ones: it isn't + sensitive, and showing it as a secret teaches the wrong thing to anyone copying the + snippet. Document how to look it up (`_edge/tenant_info`, or + `accessible-resources` for the supported route). +- `.env.example`. +- `CLAUDE.md`: Configuration section, keeping the "token is deliberately never a + flag" note (still true), and record the scope table above so it is not re-derived. + +## Commits + +1. `feat(client): route requests through the API gateway when a cloud ID is set` — + config, two bases, `SiteURL()` at all call sites, `resolveNext`, tests. +2. `docs: document scoped service-account tokens` — README, `.env.example`, CLAUDE.md. diff --git a/cmd/create/create.go b/cmd/create/create.go index 7f9e888..696033f 100644 --- a/cmd/create/create.go +++ b/cmd/create/create.go @@ -90,8 +90,11 @@ func run(cmd *cobra.Command, args []string) error { url, _ := cmd.Flags().GetString("url") username, _ := cmd.Flags().GetString("username") + cloudID, _ := cmd.Flags().GetString("cloud-id") envFile, _ := cmd.Flags().GetString("env-file") - c, err := client.Resolve(url, username, envFile) + c, err := client.Resolve(client.Options{ + URL: url, Username: username, CloudID: cloudID, EnvFile: envFile, + }) if err != nil { return fatalFail(err.Error(), jsonout.CodeConfig) } @@ -383,7 +386,9 @@ func createOne(r record, parentID string, c *client.ConfluenceClient, persist bo res := newResult(r) res.parent = nullableStr(parentID) - pageContent, err := convert.MdToConfluence(r.mdfile, c.BaseURL(), r.spaceKey, buildinfo.Stamp()) + // SiteURL, not BaseURL: rewritten links are published into the page, so they + // must point at the site even when requests go through the gateway. + pageContent, err := convert.MdToConfluence(r.mdfile, c.SiteURL(), r.spaceKey, buildinfo.Stamp()) if err != nil { return res.fail(err, jsonout.CodeConvert) } @@ -482,11 +487,11 @@ func resolveWidth(cliPageWidth string, fm map[string]string) (pagewidth.Width, e func pageURL(c *client.ConfluenceClient, page *client.Page, pageID string) string { if page.Links.WebUI == "" { - return fmt.Sprintf("%s/wiki/pages/viewpage.action?pageId=%s", c.BaseURL(), pageID) + return fmt.Sprintf("%s/wiki/pages/viewpage.action?pageId=%s", c.SiteURL(), pageID) } base := page.Links.Base if base == "" { - base = c.BaseURL() + "/wiki" + base = c.SiteURL() + "/wiki" } return base + page.Links.WebUI } diff --git a/cmd/create/create_test.go b/cmd/create/create_test.go index eafbd9e..49c2f00 100644 --- a/cmd/create/create_test.go +++ b/cmd/create/create_test.go @@ -3,6 +3,7 @@ package create import ( "testing" + "github.com/mozilla/markfluence/internal/client" "github.com/mozilla/markfluence/internal/frontmatter" "github.com/mozilla/markfluence/internal/pagewidth" ) @@ -76,3 +77,39 @@ func TestOverrideNeedsSingleFile(t *testing.T) { t.Error("no --title should not trigger the guard") } } + +// In gateway mode the request base is api.atlassian.com, which must never reach a +// reader. Every pageURL branch has to resolve against the site instead. +func TestPageURLUsesSiteNotGateway(t *testing.T) { + c := client.New(client.Config{SiteURL: "https://wiki.example.net", CloudID: "abc-123"}) + + tests := []struct { + name string + base, webu string + want string + }{ + { + "no webui link falls back to the site", + "", "", + "https://wiki.example.net/wiki/pages/viewpage.action?pageId=42", + }, + { + "webui with no base joins onto the site", + "", "/spaces/ENG/pages/42/Title", + "https://wiki.example.net/wiki/spaces/ENG/pages/42/Title", + }, + { + "the API's own base is preferred when present", + "https://wiki.example.net/wiki", "/spaces/ENG/pages/42/Title", + "https://wiki.example.net/wiki/spaces/ENG/pages/42/Title", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + page := &client.Page{ID: "42", Links: client.Links{Base: tc.base, WebUI: tc.webu}} + if got := pageURL(c, page, "42"); got != tc.want { + t.Errorf("pageURL = %q, want %q", got, tc.want) + } + }) + } +} diff --git a/cmd/fix/fix.go b/cmd/fix/fix.go index 9d2c344..ce415aa 100644 --- a/cmd/fix/fix.go +++ b/cmd/fix/fix.go @@ -39,8 +39,11 @@ func init() { func run(cmd *cobra.Command, args []string) error { url, _ := cmd.Flags().GetString("url") username, _ := cmd.Flags().GetString("username") + cloudID, _ := cmd.Flags().GetString("cloud-id") envFile, _ := cmd.Flags().GetString("env-file") - c, err := client.Resolve(url, username, envFile) + c, err := client.Resolve(client.Options{ + URL: url, Username: username, CloudID: cloudID, EnvFile: envFile, + }) if err != nil { if ui.IsJSON() { _ = jsonout.EmitError(os.Stderr, "fix", err.Error(), jsonout.CodeConfig) @@ -174,7 +177,7 @@ func locatePage(fm map[string]string, c *client.ConfluenceClient) (*client.Page, fmt.Fprintf(&b, "found %d pages with title %q:", len(matches), title) for _, m := range matches { fmt.Fprintf(&b, "\n - %s: %s (%s/wiki/pages/viewpage.action?pageId=%s)", - m.ID, m.Title, c.BaseURL(), m.ID) + m.ID, m.Title, c.SiteURL(), m.ID) } b.WriteString("\nadd a page_id to the frontmatter to disambiguate") return nil, errors.New(b.String()) diff --git a/cmd/info/info.go b/cmd/info/info.go index 87c8eec..096647e 100644 --- a/cmd/info/info.go +++ b/cmd/info/info.go @@ -40,8 +40,11 @@ func init() { func run(cmd *cobra.Command, args []string) error { url, _ := cmd.Flags().GetString("url") username, _ := cmd.Flags().GetString("username") + cloudID, _ := cmd.Flags().GetString("cloud-id") envFile, _ := cmd.Flags().GetString("env-file") - c, err := client.Resolve(url, username, envFile) + c, err := client.Resolve(client.Options{ + URL: url, Username: username, CloudID: cloudID, EnvFile: envFile, + }) if err != nil { return fatalFail(err.Error(), jsonout.CodeConfig) } @@ -141,9 +144,9 @@ type report struct { func buildReport(page *client.Page, c *client.ConfluenceClient, withProps bool) report { url := page.Links.Base + page.Links.WebUI if page.Links.WebUI == "" { - url = fmt.Sprintf("%s/wiki/pages/viewpage.action?pageId=%s", c.BaseURL(), page.ID) + url = fmt.Sprintf("%s/wiki/pages/viewpage.action?pageId=%s", c.SiteURL(), page.ID) } else if page.Links.Base == "" { - url = c.BaseURL() + "/wiki" + page.Links.WebUI + url = c.SiteURL() + "/wiki" + page.Links.WebUI } cache := map[string]string{} diff --git a/cmd/read/read.go b/cmd/read/read.go index 5a4bfbf..b9fade9 100644 --- a/cmd/read/read.go +++ b/cmd/read/read.go @@ -58,8 +58,11 @@ func run(cmd *cobra.Command, args []string) error { url, _ := cmd.Flags().GetString("url") username, _ := cmd.Flags().GetString("username") + cloudID, _ := cmd.Flags().GetString("cloud-id") envFile, _ := cmd.Flags().GetString("env-file") - c, err := client.Resolve(url, username, envFile) + c, err := client.Resolve(client.Options{ + URL: url, Username: username, CloudID: cloudID, EnvFile: envFile, + }) if err != nil { return fatalFail(err.Error(), jsonout.CodeConfig) } diff --git a/cmd/root.go b/cmd/root.go index 9ff2422..c61e5c8 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -20,6 +20,7 @@ import ( var ( urlFlag string usernameFlag string + cloudIDFlag string envFileFlag string debugFlag bool noColorFlag bool @@ -31,9 +32,15 @@ var rootCmd = &cobra.Command{ Short: "Publish markdown to Confluence", Long: "markfluence publishes and manipulates Confluence pages from markdown files.\n\n" + "Configuration resolves with the precedence flag > environment variable >\n" + - ".env file. The base URL (--url / CONFLUENCE_URL) and username (--username /\n" + - "CONFLUENCE_USERNAME) may be set any of those ways; the API token\n" + - "(CONFLUENCE_TOKEN) comes only from the environment or .env, never a flag.", + ".env file. The site URL (--url / CONFLUENCE_URL), username (--username /\n" + + "CONFLUENCE_USERNAME), and cloud ID (--cloud-id / CONFLUENCE_CLOUD_ID) may be\n" + + "set any of those ways; the API token (CONFLUENCE_TOKEN) comes only from the\n" + + "environment or .env, never a flag.\n\n" + + "Set the cloud ID to authenticate with a scoped API token, such as one issued\n" + + "to a service account: those tokens are rejected against the site domain and\n" + + "must go through Atlassian's api.atlassian.com gateway. Leave it unset for an\n" + + "unscoped personal token. Find yours at\n" + + "https://YOUR-SITE.atlassian.net/_edge/tenant_info -- it isn't a secret.", // --version prints the build stamp ("markfluence VERSION (SHA, DATE)"), the // same string the converter substitutes for the // token. @@ -102,6 +109,9 @@ func init() { "Confluence base URL (falls back to $CONFLUENCE_URL, then .env)") rootCmd.PersistentFlags().StringVar(&usernameFlag, "username", "", "Confluence username/email (falls back to $CONFLUENCE_USERNAME, then .env)") + rootCmd.PersistentFlags().StringVar(&cloudIDFlag, "cloud-id", "", + "Atlassian cloud ID; set to use a scoped API token via the api.atlassian.com "+ + "gateway (falls back to $CONFLUENCE_CLOUD_ID, then .env)") rootCmd.PersistentFlags().StringVar(&envFileFlag, "env-file", "", "Path to an env file to read (default: ./.env in the working directory)") rootCmd.PersistentFlags().BoolVarP(&debugFlag, "debug", "d", false, diff --git a/cmd/update/update.go b/cmd/update/update.go index 5a551fa..9e68b0f 100644 --- a/cmd/update/update.go +++ b/cmd/update/update.go @@ -63,8 +63,11 @@ func run(cmd *cobra.Command, args []string) error { url, _ := cmd.Flags().GetString("url") username, _ := cmd.Flags().GetString("username") + cloudID, _ := cmd.Flags().GetString("cloud-id") envFile, _ := cmd.Flags().GetString("env-file") - c, err := client.Resolve(url, username, envFile) + c, err := client.Resolve(client.Options{ + URL: url, Username: username, CloudID: cloudID, EnvFile: envFile, + }) if err != nil { if ui.IsJSON() { _ = jsonout.EmitError(os.Stderr, "update", err.Error(), jsonout.CodeConfig) @@ -156,7 +159,9 @@ func processFile(filename string, c *client.ConfluenceClient) *updateResult { } } - pageContent, err := convert.MdToConfluence(mf, c.BaseURL(), r.space, buildinfo.Stamp()) + // SiteURL, not BaseURL: rewritten links are published into the page, so they + // must point at the site even when requests go through the gateway. + pageContent, err := convert.MdToConfluence(mf, c.SiteURL(), r.space, buildinfo.Stamp()) if err != nil { return r.fail(err, jsonout.CodeConvert) } @@ -281,11 +286,11 @@ func resolveWidth(cliPageWidth string, mf *frontmatter.MarkdownFile) (pagewidth. func pageURL(c *client.ConfluenceClient, page *client.Page, pageID string) string { if page.Links.WebUI == "" { - return fmt.Sprintf("%s/wiki/pages/viewpage.action?pageId=%s", c.BaseURL(), pageID) + return fmt.Sprintf("%s/wiki/pages/viewpage.action?pageId=%s", c.SiteURL(), pageID) } base := page.Links.Base if base == "" { - base = c.BaseURL() + "/wiki" + base = c.SiteURL() + "/wiki" } return base + page.Links.WebUI } diff --git a/cmd/update/update_test.go b/cmd/update/update_test.go index 4c05e59..9b2cc81 100644 --- a/cmd/update/update_test.go +++ b/cmd/update/update_test.go @@ -3,6 +3,7 @@ package update import ( "testing" + "github.com/mozilla/markfluence/internal/client" "github.com/mozilla/markfluence/internal/frontmatter" "github.com/mozilla/markfluence/internal/pagewidth" ) @@ -91,3 +92,39 @@ func TestOverrideNeedsSingleFile(t *testing.T) { }) } } + +// In gateway mode the request base is api.atlassian.com, which must never reach a +// reader. Every pageURL branch has to resolve against the site instead. +func TestPageURLUsesSiteNotGateway(t *testing.T) { + c := client.New(client.Config{SiteURL: "https://wiki.example.net", CloudID: "abc-123"}) + + tests := []struct { + name string + base, webu string + want string + }{ + { + "no webui link falls back to the site", + "", "", + "https://wiki.example.net/wiki/pages/viewpage.action?pageId=42", + }, + { + "webui with no base joins onto the site", + "", "/spaces/ENG/pages/42/Title", + "https://wiki.example.net/wiki/spaces/ENG/pages/42/Title", + }, + { + "the API's own base is preferred when present", + "https://wiki.example.net/wiki", "/spaces/ENG/pages/42/Title", + "https://wiki.example.net/wiki/spaces/ENG/pages/42/Title", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + page := &client.Page{ID: "42", Links: client.Links{Base: tc.base, WebUI: tc.webu}} + if got := pageURL(c, page, "42"); got != tc.want { + t.Errorf("pageURL = %q, want %q", got, tc.want) + } + }) + } +} diff --git a/internal/client/client.go b/internal/client/client.go index be62c05..eb26973 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -4,6 +4,16 @@ // Requests are built as absolute URLs off the base URL. Pages and content // properties use the Confluence v2 API; attachment writes and the user lookup // use v1 (/wiki/rest/api/...) since v2 doesn't cover them. +// +// A client carries two bases. baseURL is where requests go; siteURL is the +// human-facing site the pages live on. They differ only when a cloud ID selects +// the platform API gateway (see Config): a scoped API token -- the kind a service +// account gets -- is rejected against the site domain and must go through +// https://api.atlassian.com/ex/confluence/{cloudId} instead. The path suffixes are +// identical under the gateway, so every call below is written against baseURL +// unchanged. siteURL exists because the gateway host must never reach a reader: +// it's wrong in printed URLs and, worse, would be written into published page +// content by the converter's link rewriting. package client import ( @@ -49,27 +59,60 @@ const ( // sleep is the backoff pause primitive; a package variable so tests can stub it. var sleep = time.Sleep +// gatewayPrefix is the platform API gateway a scoped token must use, joined with +// the cloud ID to form the request base. +const gatewayPrefix = "https://api.atlassian.com/ex/confluence/" + // ConfluenceClient talks to the Confluence REST API as a single authenticated user. type ConfluenceClient struct { - baseURL string + baseURL string // where requests go: the gateway when a cloud ID is set, else the site + siteURL string // always the Confluence site, for URLs a reader will see username string token string http *http.Client } -// New builds a client for baseURL authenticating as username:token. -func New(baseURL, username, token string) *ConfluenceClient { +// Config holds what a client needs to reach Confluence. The fields are named +// rather than positional because SiteURL and CloudID both address the same site +// and Token is a secret; transposing them would be easy and the failure obscure. +type Config struct { + // SiteURL is the Confluence site, e.g. https://your-org.atlassian.net. + SiteURL string + // CloudID, when set, routes requests through the platform API gateway. Leave + // it empty for site-domain requests, which is what an unscoped personal token + // and any Data Center site need. + CloudID string + // Username is the account the token belongs to (basic auth). + Username string + // Token is the API token. + Token string +} + +// New builds a client from cfg. +func New(cfg Config) *ConfluenceClient { + site := strings.TrimRight(cfg.SiteURL, "/") + base := site + if cfg.CloudID != "" { + base = gatewayPrefix + cfg.CloudID + } return &ConfluenceClient{ - baseURL: strings.TrimRight(baseURL, "/"), - username: username, - token: token, + baseURL: base, + siteURL: site, + username: cfg.Username, + token: cfg.Token, http: &http.Client{}, } } -// BaseURL returns the client's base URL (trailing slash trimmed). +// BaseURL returns the base requests are built off (trailing slash trimmed). This +// is the gateway when a cloud ID is configured, so it must not be shown to a +// reader or written into page content -- use SiteURL for that. func (c *ConfluenceClient) BaseURL() string { return c.baseURL } +// SiteURL returns the Confluence site (trailing slash trimmed), regardless of +// whether requests are routed through the gateway. +func (c *ConfluenceClient) SiteURL() string { return c.siteURL } + // HTTPError is returned when the API responds with a >= 400 status. type HTTPError struct { StatusCode int @@ -637,6 +680,29 @@ func (c *ConfluenceClient) GetContentProperty(pageID, key string) (*Property, er return &out.Results[0], nil } +// resolveNext turns a paginated response's _links.next into an absolute URL +// against base, returning "" when there is no next page. +// +// next is normally a site-relative absolute path ("/wiki/api/v2/..."), so it is +// appended to base -- which is what preserves the gateway's /ex/confluence/{cloudId} +// segment. url.ResolveReference would be wrong here: an absolute-path reference +// replaces the whole path and would silently drop that prefix. The middle branch +// guards the converse, should the gateway ever echo next with the prefix already +// applied, which plain appending would double. +func resolveNext(base, next string) string { + switch { + case next == "": + return "" + case strings.HasPrefix(next, "http://"), strings.HasPrefix(next, "https://"): + return next + } + if u, err := url.Parse(base); err == nil && u.Path != "" && strings.HasPrefix(next, u.Path) { + u.Path, u.RawQuery, u.Fragment = "", "", "" + return strings.TrimRight(u.String(), "/") + next + } + return base + next +} + // ListContentProperties returns all of a page's content properties, following // pagination. func (c *ConfluenceClient) ListContentProperties(pageID string) ([]Property, error) { @@ -655,11 +721,7 @@ func (c *ConfluenceClient) ListContentProperties(pageID string) ([]Property, err } results = append(results, out.Results...) // The next link already carries the cursor and limit as query params. - if out.Links.Next != "" { - rawURL = c.baseURL + out.Links.Next - } else { - rawURL = "" - } + rawURL = resolveNext(c.baseURL, out.Links.Next) params = nil } return results, nil diff --git a/internal/client/client_test.go b/internal/client/client_test.go index f2d1e71..7435afd 100644 --- a/internal/client/client_test.go +++ b/internal/client/client_test.go @@ -44,7 +44,7 @@ func newServer(t *testing.T, responses ...resp) (*ConfluenceClient, *scripted) { _, _ = w.Write([]byte(out.body)) })) t.Cleanup(srv.Close) - return New(srv.URL, "u", "t"), s + return New(Config{SiteURL: srv.URL, Username: "u", Token: "t"}), s } func eqStrings(a, b []string) bool { @@ -137,7 +137,7 @@ func countingServer(t *testing.T, handler func(w http.ResponseWriter, n int32)) handler(w, atomic.AddInt32(&n, 1)) })) t.Cleanup(srv.Close) - return New(srv.URL, "u", "t"), &n + return New(Config{SiteURL: srv.URL, Username: "u", Token: "t"}), &n } func TestSendRetriesOn429ThenSucceeds(t *testing.T) { @@ -471,23 +471,25 @@ func TestResolve(t *testing.T) { "CONFLUENCE_TOKEN=file-pass\n"), 0o644); err != nil { t.Fatal(err) } - // Clear any inherited env for a deterministic baseline. + // Clear any inherited env for a deterministic baseline. CONFLUENCE_CLOUD_ID + // matters here too: a stray one would reroute BaseURL to the gateway. t.Setenv("CONFLUENCE_URL", "") t.Setenv("CONFLUENCE_USERNAME", "") t.Setenv("CONFLUENCE_TOKEN", "") + t.Setenv("CONFLUENCE_CLOUD_ID", "") // All from .env. - c, err := Resolve("", "", "") + c, err := Resolve(Options{}) if err != nil || c.BaseURL() != "https://file.example.net" { t.Fatalf("Resolve(.env) = %v, %v", c, err) } // Flag beats env beats .env for the URL. t.Setenv("CONFLUENCE_URL", "https://env.example.net") - if c, _ := Resolve("https://flag.example.net", "", ""); c.BaseURL() != "https://flag.example.net" { + if c, _ := Resolve(Options{URL: "https://flag.example.net"}); c.BaseURL() != "https://flag.example.net" { t.Errorf("flag should win, got %q", c.BaseURL()) } - if c, _ := Resolve("", "", ""); c.BaseURL() != "https://env.example.net" { + if c, _ := Resolve(Options{}); c.BaseURL() != "https://env.example.net" { t.Errorf("env should beat .env, got %q", c.BaseURL()) } @@ -496,7 +498,78 @@ func TestResolve(t *testing.T) { if err := os.WriteFile(".env", []byte("CONFLUENCE_URL=u\nCONFLUENCE_USERNAME=x\n"), 0o644); err != nil { t.Fatal(err) } - if _, err := Resolve("u", "x", ""); err == nil { + if _, err := Resolve(Options{URL: "u", Username: "x"}); err == nil { t.Error("Resolve with no token: want error") } } + +func TestNewGatewayBase(t *testing.T) { + tests := []struct { + name string + site, cloudID string + wantBase, wantSite string + }{ + { + "no cloud ID keeps the site as the request base", + "https://wiki.example.net", "", + "https://wiki.example.net", "https://wiki.example.net", + }, + { + "cloud ID routes requests to the gateway, site unchanged", + "https://wiki.example.net", "abc-123", + gatewayPrefix + "abc-123", "https://wiki.example.net", + }, + { + "trailing slash trimmed from both", + "https://wiki.example.net/", "abc-123", + gatewayPrefix + "abc-123", "https://wiki.example.net", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + c := New(Config{SiteURL: tc.site, CloudID: tc.cloudID}) + if c.BaseURL() != tc.wantBase { + t.Errorf("BaseURL = %q, want %q", c.BaseURL(), tc.wantBase) + } + if c.SiteURL() != tc.wantSite { + t.Errorf("SiteURL = %q, want %q", c.SiteURL(), tc.wantSite) + } + }) + } +} + +func TestResolveNext(t *testing.T) { + const gw = gatewayPrefix + "abc-123" + tests := []struct { + name, base, next, want string + }{ + {"no next page", gw, "", ""}, + { + "site-relative path appends, preserving the gateway prefix", + gw, "/wiki/api/v2/pages/1/properties?cursor=X", + gw + "/wiki/api/v2/pages/1/properties?cursor=X", + }, + { + "prefix already applied is not doubled", + gw, "/ex/confluence/abc-123/wiki/api/v2/pages/1/properties?cursor=X", + gw + "/wiki/api/v2/pages/1/properties?cursor=X", + }, + { + "absolute next is used as-is", + gw, "https://elsewhere.example.net/wiki/api/v2/pages?cursor=X", + "https://elsewhere.example.net/wiki/api/v2/pages?cursor=X", + }, + { + "site base (no cloud ID) appends as before", + "https://wiki.example.net", "/wiki/api/v2/pages/1/properties?cursor=X", + "https://wiki.example.net/wiki/api/v2/pages/1/properties?cursor=X", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := resolveNext(tc.base, tc.next); got != tc.want { + t.Errorf("resolveNext(%q, %q) = %q, want %q", tc.base, tc.next, got, tc.want) + } + }) + } +} diff --git a/internal/client/config.go b/internal/client/config.go index 8a5032f..952301c 100644 --- a/internal/client/config.go +++ b/internal/client/config.go @@ -12,31 +12,51 @@ const ( urlEnv = "CONFLUENCE_URL" usernameEnv = "CONFLUENCE_USERNAME" tokenEnv = "CONFLUENCE_TOKEN" // the API token; never a command-line flag + cloudIDEnv = "CONFLUENCE_CLOUD_ID" dotenvPath = ".env" ) var spaceKeyRE = regexp.MustCompile(`^/spaces/([^/]+)/`) -// Resolve builds a client from the base URL, username, and token. Each value is -// resolved with the precedence flag > environment variable > .env file: the URL -// and username come from the urlFlag/usernameFlag values when set, then -// $CONFLUENCE_URL/$CONFLUENCE_USERNAME, then the .env file; the API token comes -// only from $CONFLUENCE_TOKEN, then .env -- never a flag. envFile selects which -// .env is read: when empty the default ./.env is read best-effort (a missing -// file is fine); when set it's an explicit path that must be readable. It -// returns a friendly error listing whatever is missing. -func Resolve(urlFlag, usernameFlag, envFile string) (*ConfluenceClient, error) { - env, err := loadEnvFile(envFile) +// Options carries the flag values Resolve needs, named so the two URL-ish fields +// can't be transposed at a call site. +type Options struct { + // URL is the --url value (the Confluence site). + URL string + // Username is the --username value. + Username string + // CloudID is the --cloud-id value; set it to route requests through the + // platform API gateway, which a scoped service-account token requires. + CloudID string + // EnvFile is the --env-file value; empty means the default ./.env. + EnvFile string +} + +// Resolve builds a client from the site URL, username, cloud ID, and token. Each +// value is resolved with the precedence flag > environment variable > .env file: +// the URL, username, and cloud ID come from opts when set, then +// $CONFLUENCE_URL/$CONFLUENCE_USERNAME/$CONFLUENCE_CLOUD_ID, then the .env file; +// the API token comes only from $CONFLUENCE_TOKEN, then .env -- never a flag. +// opts.EnvFile selects which .env is read: when empty the default ./.env is read +// best-effort (a missing file is fine); when set it's an explicit path that must +// be readable. It returns a friendly error listing whatever is missing. +// +// The cloud ID is optional: without one, requests go to the site domain exactly +// as before, which is what an unscoped personal token and any Data Center site +// need. +func Resolve(opts Options) (*ConfluenceClient, error) { + env, err := loadEnvFile(opts.EnvFile) if err != nil { return nil, err } - baseURL := resolveValue(urlFlag, urlEnv, env) - username := resolveValue(usernameFlag, usernameEnv, env) + siteURL := resolveValue(opts.URL, urlEnv, env) + username := resolveValue(opts.Username, usernameEnv, env) + cloudID := resolveValue(opts.CloudID, cloudIDEnv, env) token := resolveValue("", tokenEnv, env) var missing []string - if baseURL == "" { + if siteURL == "" { missing = append(missing, "URL (--url or "+urlEnv+")") } if username == "" { @@ -48,7 +68,29 @@ func Resolve(urlFlag, usernameFlag, envFile string) (*ConfluenceClient, error) { if len(missing) > 0 { return nil, errors.New("missing Confluence " + strings.Join(missing, ", ")) } - return New(baseURL, username, token), nil + if err := validateCloudID(cloudID); err != nil { + return nil, err + } + return New(Config{ + SiteURL: siteURL, + CloudID: cloudID, + Username: username, + Token: token, + }), nil +} + +// validateCloudID rejects a cloud ID that looks like a URL or a path fragment. +// The value is joined straight onto the gateway prefix, so pasting a whole +// gateway URL would otherwise produce an opaque 404 rather than a usable error. +func validateCloudID(cloudID string) error { + if cloudID == "" { + return nil + } + if strings.ContainsAny(cloudID, "/:") { + return fmt.Errorf("invalid Confluence cloud ID %q (--cloud-id or %s): expected just the "+ + "identifier, not a URL or path", cloudID, cloudIDEnv) + } + return nil } // resolveValue applies the flag > environment > .env precedence for one setting. diff --git a/internal/client/config_test.go b/internal/client/config_test.go index 2e4872d..9337cba 100644 --- a/internal/client/config_test.go +++ b/internal/client/config_test.go @@ -11,7 +11,7 @@ import ( // the only sources. func clearConfluenceEnv(t *testing.T) { t.Helper() - for _, k := range []string{urlEnv, usernameEnv, tokenEnv} { + for _, k := range []string{urlEnv, usernameEnv, tokenEnv, cloudIDEnv} { t.Setenv(k, "") } } @@ -28,7 +28,7 @@ func writeEnvFile(t *testing.T, body string) string { func TestResolveUsesExplicitEnvFile(t *testing.T) { clearConfluenceEnv(t) path := writeEnvFile(t, "CONFLUENCE_URL=https://wiki\nCONFLUENCE_USERNAME=bot\nCONFLUENCE_TOKEN=secret\n") - c, err := Resolve("", "", path) + c, err := Resolve(Options{EnvFile: path}) if err != nil { t.Fatalf("Resolve: %v", err) } @@ -40,7 +40,7 @@ func TestResolveUsesExplicitEnvFile(t *testing.T) { func TestResolveFlagOverridesEnvFile(t *testing.T) { clearConfluenceEnv(t) path := writeEnvFile(t, "CONFLUENCE_URL=https://from-file\nCONFLUENCE_USERNAME=bot\nCONFLUENCE_TOKEN=secret\n") - c, err := Resolve("https://from-flag", "", path) + c, err := Resolve(Options{URL: "https://from-flag", EnvFile: path}) if err != nil { t.Fatalf("Resolve: %v", err) } @@ -52,7 +52,7 @@ func TestResolveFlagOverridesEnvFile(t *testing.T) { func TestResolveMissingExplicitEnvFileErrors(t *testing.T) { clearConfluenceEnv(t) missing := filepath.Join(t.TempDir(), "nope.env") - if _, err := Resolve("", "", missing); err == nil { + if _, err := Resolve(Options{EnvFile: missing}); err == nil { t.Fatal("Resolve: want error for a missing --env-file path") } } @@ -62,7 +62,7 @@ func TestResolveDefaultEnvFileMissingIsFine(t *testing.T) { // No ./.env in this temp cwd, and no explicit env file: the missing default // is tolerated, so we fail only on missing config values (not a read error). t.Chdir(t.TempDir()) - _, err := Resolve("", "", "") + _, err := Resolve(Options{}) if err == nil { t.Fatal("want a missing-config error") } @@ -71,3 +71,66 @@ func TestResolveDefaultEnvFileMissingIsFine(t *testing.T) { t.Errorf("error = %q, want a missing-Confluence-config error", err) } } + +func TestResolveCloudIDPrecedence(t *testing.T) { + clearConfluenceEnv(t) + path := writeEnvFile(t, + "CONFLUENCE_URL=https://wiki\nCONFLUENCE_USERNAME=bot\nCONFLUENCE_TOKEN=secret\n"+ + "CONFLUENCE_CLOUD_ID=from-file\n") + + // From .env: requests move to the gateway, the site is untouched. + c, err := Resolve(Options{EnvFile: path}) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + if want := gatewayPrefix + "from-file"; c.BaseURL() != want { + t.Errorf("BaseURL = %q, want %q", c.BaseURL(), want) + } + if c.SiteURL() != "https://wiki" { + t.Errorf("SiteURL = %q, want https://wiki", c.SiteURL()) + } + + // Env beats .env; flag beats env. + t.Setenv(cloudIDEnv, "from-env") + if c, _ := Resolve(Options{EnvFile: path}); c.BaseURL() != gatewayPrefix+"from-env" { + t.Errorf("env should beat .env, got %q", c.BaseURL()) + } + if c, _ := Resolve(Options{CloudID: "from-flag", EnvFile: path}); c.BaseURL() != gatewayPrefix+"from-flag" { + t.Errorf("flag should win, got %q", c.BaseURL()) + } +} + +func TestResolveRejectsURLishCloudID(t *testing.T) { + clearConfluenceEnv(t) + path := writeEnvFile(t, "CONFLUENCE_URL=https://wiki\nCONFLUENCE_USERNAME=bot\nCONFLUENCE_TOKEN=secret\n") + + // Pasting a whole gateway URL (or any path fragment) is the likely mistake; + // it must fail with a usable message rather than a 404 at request time. + for _, bad := range []string{ + "https://api.atlassian.com/ex/confluence/abc", + "ex/confluence/abc", + "abc/wiki", + } { + _, err := Resolve(Options{CloudID: bad, EnvFile: path}) + if err == nil { + t.Errorf("Resolve(cloud ID %q): want an error", bad) + continue + } + if !strings.Contains(err.Error(), "invalid Confluence cloud ID") { + t.Errorf("Resolve(cloud ID %q) error = %q, want the invalid-cloud-ID message", bad, err) + } + } +} + +func TestResolveWithoutCloudIDKeepsSiteURL(t *testing.T) { + clearConfluenceEnv(t) + path := writeEnvFile(t, "CONFLUENCE_URL=https://wiki/\nCONFLUENCE_USERNAME=bot\nCONFLUENCE_TOKEN=secret\n") + c, err := Resolve(Options{EnvFile: path}) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + // No cloud ID: both bases are the site, exactly as before the gateway existed. + if c.BaseURL() != "https://wiki" || c.SiteURL() != "https://wiki" { + t.Errorf("BaseURL/SiteURL = %q/%q, want https://wiki for both", c.BaseURL(), c.SiteURL()) + } +}