Skip to content

ESD-1688: Act on behalf of a managed account - #547

Open
Phil-Browne wants to merge 3 commits into
mainfrom
worktree-esd-1688-on-behalf-of-managed-account
Open

ESD-1688: Act on behalf of a managed account#547
Phil-Browne wants to merge 3 commits into
mainfrom
worktree-esd-1688-on-behalf-of-managed-account

Conversation

@Phil-Browne

@Phil-Browne Phil-Browne commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Adds a global --on-behalf-of flag (and a MEGAPORT_MANAGED_ACCOUNT_UID env fallback) that names a managed account company UID. When set, the authenticated client is built with megaport.WithCallContext(uid), so every authenticated API call for that invocation carries X-Call-Context: <uid>. Unset means no header and unchanged behavior. This is the CLI half of EIP-7108 (ESD-1610 shipped the SDK option; ESD-1611 is the Terraform equivalent).

Follows the existing --base-url / utils.BaseURL pattern: flag first, then env var.

Changes:

  • Register the --on-behalf-of persistent flag on both the native and WASM root commands.
  • Resolve the UID (flag, then env) in one shared helper and append WithCallContext when non-empty, on both native login and both WASM login branches (portal token and API-key).
  • WASM setAuthCredentials gains an optional 4th arg for the UID; clearAuthCredentials and setAuthToken clear it so it never leaks across auth sessions.
  • Login now surfaces active on-behalf-of mode: native prints "Acting on behalf of managed account " alongside the success message, and both WASM login branches log the same to the console, so a resolved UID is never silently invisible.
  • Tests assert the header is present with the resolved UID and absent when nothing is set, plus the WASM env wiring.

Sending the header on every authenticated request is correct: verified in the megalith backend that X-Call-Context is consumed by the global SessionTokenAuthenticationFilter (not per-endpoint), which swaps the session's company to the managed child before any controller runs, so all authenticated endpoints honor it. The backend enforces authorization (canManageChild returns 403 if the caller can't manage that company) and rejects the header for assumed-role sessions, so forwarding a UID is safe. The OpenAPI spec only declaring the parameter on three pricebook GETs is incomplete documentation, not the real scope.

What I could not verify

  • The real in-browser request carrying the header can't be exercised in CI (no fetch under node), a known limitation for this package. The WithCallContext option application is proven by the native request-level test (same SDK code path); the WASM flag/env wiring is unit-tested, but the actual browser request is verified manually in-browser.
  • WithCallContext sets the header on the client before Authorize() runs, so the token-exchange request to the auth-m2m host also carries X-Call-Context, not just resource API calls. This isn't new: the existing WithCustomHeaders(cliHeaders) option already does the same, unchanged by this PR, so no new host is receiving unexpected headers. Standard HTTP servers ignore headers they don't recognize and the token endpoint has no company-context concept to act on, so risk reads as low, but the token-issuing service is a separate microservice consumed only as an external dependency, with no source in this megalith checkout to confirm it's ignored outright.

Add a global --on-behalf-of flag and MEGAPORT_MANAGED_ACCOUNT_UID env
fallback naming a managed account company UID. When set, the authenticated
Megaport client is built with megaport.WithCallContext(uid) so every
authenticated API call carries the X-Call-Context header; unset sends no
header. Wired through both native and WASM auth paths (token and credentials),
plus the setAuthCredentials optional 4th arg.
Copilot AI review requested due to automatic review settings July 23, 2026 11:46
@Phil-Browne
Phil-Browne requested review from a team and penzeliz-megaport as code owners July 23, 2026 11:46
@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.49%. Comparing base (f266dc2) to head (f63cbb2).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #547      +/-   ##
==========================================
- Coverage   79.50%   79.49%   -0.01%     
==========================================
  Files         193      193              
  Lines       18859    18870      +11     
==========================================
+ Hits        14993    15001       +8     
- Misses       2818     2820       +2     
- Partials     1048     1049       +1     
Files with missing lines Coverage Δ
cmd/megaport/megaport_common.go 100.00% <100.00%> (ø)
internal/commands/config/config_shared.go 93.10% <100.00%> (+1.10%) ⬆️
internal/commands/config/login.go 88.26% <100.00%> (+0.34%) ⬆️
internal/utils/utils.go 95.93% <ø> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds support for acting on behalf of a managed account across the CLI by introducing a global --on-behalf-of flag (with MEGAPORT_MANAGED_ACCOUNT_UID env fallback) and applying megaport.WithCallContext(uid) during client construction so authenticated requests carry X-Call-Context: <uid>.

Changes:

  • Added global --on-behalf-of persistent flag (native + WASM) and a shared resolver (flag first, then env var).
  • Applied megaport.WithCallContext(uid) to native login and both WASM authenticated login branches (portal token + API key/secret).
  • Extended WASM setAuthCredentials to accept an optional UID and ensured UID is cleared on credential/token/session transitions; updated tests, types, and docs accordingly.

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
internal/wasm/wasm.go Adds managed-account UID handling to WASM credential/token lifecycle (set/clear) via env var.
internal/wasm/wasm_test.go Adds unit tests covering UID set/clear behavior and undefined/null handling.
internal/utils/utils.go Introduces utils.ManagedAccountUID global bound to the new persistent flag.
internal/commands/config/login.go Applies WithCallContext during native authenticated client construction.
internal/commands/config/login_wasm.go Applies WithCallContext in both WASM authenticated client creation paths.
internal/commands/config/login_test.go Adds request-level test asserting presence/absence of X-Call-Context.
internal/commands/config/config_shared.go Adds shared UID resolver (flag first, then env var).
frontend-integration/types/megaport-wasm.d.ts Updates TS types for setAuthCredentials(..., managedAccountUID?).
docs/megaport-cli.md Documents the new global --on-behalf-of flag.
cmd/megaport/megaport_common.go Registers --on-behalf-of on the native root command.
cmd/megaport/common_wasm.go Registers --on-behalf-of on the WASM root command.

Comment thread internal/commands/config/config_shared.go
Fixes the threat-model finding that a resolved --on-behalf-of UID was
silently invisible: a stale or forgotten flag could leave someone
acting on a managed account without realizing it. Native login now
prints "Acting on behalf of managed account <uid>" alongside the
success message; both WASM login branches log the same to the
console. Also drops the now-redundant uid != "" guard on the native
path, since WithCallContext already no-ops on an empty string.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants