Add alternativeWebUrl param#956
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e9e8beed19
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const browserUrl = new URL( | ||
| resolved.pathname + resolved.search + resolved.hash, | ||
| browserBase, | ||
| ); |
There was a problem hiding this comment.
When coder.alternativeWebUrl contains a path prefix (for example a reverse proxy at https://proxy.example.com/coder), constructing new URL(resolved.pathname..., browserBase) drops that prefix because the first argument starts with /; a chat navigation to /templates opens https://proxy.example.com/templates instead of https://proxy.example.com/coder/templates. Other browser links concatenate onto the resolved base and preserve such prefixes, so this makes chat links fail only for path-based alternative web URLs.
Useful? React with 👍 / 👎.
| await vscode.env.openExternal( | ||
| vscode.Uri.parse(`${resolveBrowserUrl(url)}/cli-auth`), | ||
| ); |
There was a problem hiding this comment.
This only switches the legacy token page to coder.alternativeWebUrl; when coder.experimental.oauth is enabled and the user chooses OAuth, loginWithOAuth still goes through OAuthAuthorizer.startAuthorization, which opens the discovered authorization URL directly. In deployments where the connection URL uses a browser-restricted/unreachable port and the web UI is available via the alternative URL, OAuth login still opens the restricted connection URL and cannot complete, while token login works.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
We didn't touch the OAuth path deliberately, but I think the case highlighted here is worth addressing so we have consistency. Will submit a fix shortly.
EhabY
left a comment
EhabY
left a comment
There was a problem hiding this comment.
Nice improvement. Make sure to run pnpm format and pnpm lint:fix/pnpm typecheck!
| * `coder.alternativeWebUrl` setting when configured, otherwise returns | ||
| * the connection URL unchanged. | ||
| */ | ||
| export function resolveBrowserUrl(connectionUrl: string): string { |
There was a problem hiding this comment.
resolveUiUrl makes more sense to me and fits the setting better coder.alternativeWebUrl
| // The server-advertised endpoint is authoritative for the path, but the | ||
| // origin may be unreachable from a browser (e.g. blocked port). When | ||
| // `coder.alternativeWebUrl` is set, swap the origin so the user lands on | ||
| // a reachable host while preserving the path the server told us to use. |
There was a problem hiding this comment.
Agents tend to over explain, I think this can be trimmed or removed entirely
| // Concatenate rather than `new URL(path, base)` so a path prefix on | ||
| // the alternative URL (e.g. a reverse proxy at https://host/coder) | ||
| // is preserved. |
There was a problem hiding this comment.
// Preserve the server-advertised path; only the origin/prefix may change.
| const baseUrl = resolveBrowserUrl(connUrl); | ||
| const task = await this.client.getTask("me", taskId); | ||
| vscode.env.openExternal(vscode.Uri.parse(getTaskBuildUrl(baseUrl, task))); |
There was a problem hiding this comment.
Instead of always doing resolveXUrl then vscode.open or vscode.env.openExternal, how about we just add openInBrowser(connectionUrl, path) helper
There was a problem hiding this comment.
We could even do it in a safer way like vscode.Uri.joinPath + Uri.with({ query, fragment }) so that we do not have to use some regex magic and concatenation
| vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({ | ||
| get: vi.fn(), |
There was a problem hiding this comment.
Do use MockConfigurationProvider instead of mocking this directly (here and in tasksPanelProvider.test.ts
| "default": "" | ||
| }, | ||
| "coder.alternativeWebUrl": { | ||
| "markdownDescription": "An alternative URL to use when opening Coder pages in the browser. When set, this replaces the connection URL for browser links only (dashboard, workspace pages, token authentication page, OAuth authorization page). The connection URL is still used for API calls, SSH, and CLI operations. Useful when the Coder API runs on a port that browsers restrict (e.g., 7004) but the web UI is accessible on a standard port (e.g., 443).", |
There was a problem hiding this comment.
Do add something like "when empty, the connection URL is used for UI". I do think we need to be clear that this only affects calls like vscode.env.openExternal (without mentioning this here of course)
| const endpoint = new URL(metadata.authorization_endpoint); | ||
| const browserBase = resolveBrowserUrl(endpoint.origin); | ||
| const url = `${browserBase}${endpoint.pathname}?${params.toString()}`; | ||
|
|
There was a problem hiding this comment.
The old code was:
const url = `${metadata.authorization_endpoint}?${params.toString()}`;The new code drops endpoint.search and passes endpoint.origin (not a full connection URL) into the helper, which is inconsistent with every other call site. Preserve the original shape and just swap the origin:
const endpoint = new URL(metadata.authorization_endpoint);
const browserOrigin = new URL(resolveUiUrl(coderApi.getHost())).origin;
endpoint.protocol = new URL(browserOrigin).protocol;
endpoint.host = new URL(browserOrigin).host;
for (const [key, value] of Object.entries(params)) {
endpoint.searchParams.set(key, value);
}
const url = endpoint.toString();Or, if the helper is as per the suggestion above, just call the open helper with the endpoint pathname and the params as the query. Either way, query strings already on authorization_endpoint are preserved and the helper receives a real connection URL
2 participants
This PR adds a new parameter for specifying an alternative URL to use when opening Coder pages in the browser. When set, it replaces the connection URL for browser links only (dashboard, workspace pages, token authentication page). The connection URL is still used for API calls, SSH, and CLI operations. Useful when the Coder API runs on a port that browsers restrict (e.g., 7004) but the web UI is accessible on a standard port (e.g., 443)."