Skip to content

Fixes unhandled v1 prefixed 'web/config.js' path#51364

Merged
kimlisa merged 2 commits intomasterfrom
lisa/fix-api-path
Jan 22, 2025
Merged

Fixes unhandled v1 prefixed 'web/config.js' path#51364
kimlisa merged 2 commits intomasterfrom
lisa/fix-api-path

Conversation

@kimlisa
Copy link
Copy Markdown
Contributor

@kimlisa kimlisa commented Jan 22, 2025

fixes #51362

starting v17.2.0, PR #50472 goes into affect where the lib/web/apiserver.go:NewHandler now checks for a fixed list of known paths after a /v1 prefix, before stripping of the /v1.

Teleport Connect users (with versions below v17.2.0) were unable to login to clusters with versions v17.2.0, because connect calls this endpoint web/config.js with v1 prefixed, where web wasn't part of the list of known paths so /v1/web/config.js didn't get handled

The WebClient.endpoint func would auto prefix endpoints with v1 because that's how it was configured. #50472 removes this configuration and i missed the web/config

I went through all use cases of webclient.endpoint to make sure no other paths were unhandled. Tested fix with branch/v16 connect and master cluster

changelog: Fix backwards compatibility error where users were unable to login with Teleport Connect if Connect version is below v17.2.0 with Teleport cluster version v17.2.0

Comment thread lib/web/apiserver.go
// part[1] is the prefix "v1"
switch pathParts[2] {
case "webapi", "enterprise", "scripts", ".well-known", "workload-identity":
case "webapi", "enterprise", "scripts", ".well-known", "workload-identity", "web":
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.

are we sure we want to include every request that is prefixed with web or only web/config.js?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

i think it's fine because we are just saying if /v1/web strip the /v1

@kimlisa kimlisa added this pull request to the merge queue Jan 22, 2025
Merged via the queue into master with commit 5c043d7 Jan 22, 2025
@kimlisa kimlisa deleted the lisa/fix-api-path branch January 22, 2025 19:02
@public-teleport-github-review-bot
Copy link
Copy Markdown

@kimlisa See the table below for backport results.

Branch Result
branch/v17 Create PR

@ravicious
Copy link
Copy Markdown
Member

ravicious commented Jan 23, 2025

I know that some on-call devs use /web/config.js to debug customer issues, since it returns data not included in /webapi/ping. If we ever get around to deprecating it in some way, it should be adequately messaged.

At the last offsite I've been to, I've also briefly talked with Hugo and Tiago about refactoring it to return plain JS, because now it essentially allows any code to be injected into the Web UI.

carloscastrojumo pushed a commit to carloscastrojumo/teleport that referenced this pull request Feb 19, 2025
* Fixes bug where /v1/web/config.js wasn't properly handled because of v1 prefix

* Fix lint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unable to retrieve cluster auth preferences error with Teleport Connect

5 participants