Provide a dedicated API endpoint for app FQDN resolving #6449
Conversation
Currently, an app's target FQDN can be obtained only using the endpoint for creating new app sessions. The OAuth-style back-and-forth redirects between the app launcher and the app itself are therefore forced to generate an unnecessary additional app session just to resolve the FQDN. The new endpoint introduced here allows to resolve such FQDNs by invoking a dedicated endpoint.
andrejtokarcik
requested review from
alex-kovoy,
klizhentas,
r0mant and
russjones
as code owners
lib/web/apiserver.go
Outdated
| h.GET("/webapi/apps/:fqdn", h.WithAuth(h.getAppFQDN)) | ||
| h.GET("/webapi/apps/:fqdn/:clusterName/:publicAddr", h.WithAuth(h.getAppFQDN)) |
There was a problem hiding this comment.
since this endpoint is for resolving FQDN, drop the fqdn parameter
also, the first endpoint here seems redundant?
There was a problem hiding this comment.
The fqdn parameter is needed, serves as a hint to resolve the actual/safe FQDN. It's the same logic as in the original createAppSession handler.
No, both endpoints may get used from the webapps app launcher.
There was a problem hiding this comment.
Can you show an example usage of this endpoint (example URL and response)?
I'm not familiar with how this FQDN resolution works, it just seems odd that it takes an FQDN and returns one too. Maybe the parameter should be named differently, like appName?
There was a problem hiding this comment.
Say there is a Teleport proxy running at proxy.example.com.
With the new getAppFQDN endpoint, the following would happen in an ordinary, best-case scenario:
- The user opens an app via the Web UI.
- A new tab is opened with the app launcher
https://proxy.example.com/web/launch/app.proxy.example.com/cluster-name/app.proxy.example.com(matching this URL pattern). - Since the user-provided FQDN shouldn't be blindly trusted, the app launcher invokes
getAppFQDNto get a known/safe FQDN for the app. In this casegetAppFQDNwould return the sameapp.proxy.example.comwhich is expected since the web UI populates the app launcher params based on the same data as those used bygetAppFQDN. - The app launcher would then redirect the user to
https://app.proxy.example.com/x-teleport-authwhich in turn redirects back to the app launcher with astatevalue etc. etc.
Now, there is another way to invoke the app launcher instead of going directly through Teleport Web UI:
- The user opens
whatever.proxy.example.comin his browser. - This triggers the app launcher
https://proxy.example.com/web/launch/whatever.proxy.example.com(only the FQDN param is now present, since the target cluster and the app's public address aren't known at this point). getAppFQDNguesses the intended app (from among those running in the root as well as the leaf clusters) on a best-effort basis and either fails or returns the found FQDN. If the app is configured with a public address (here let's sayapp.proxy-example.org), it could be completely unrelated to the FQDN input.- Etc.
From the robustness/security standpoint, getAppFQDN serves to prevent infinite redirects and block malicious FQDN inputs (e.g., stealing valid app sessions, phishing). This was already achieved with CreateAppSession which also returns the FQDN as part of its output but with the undesirable side effect of creating a new valid app session (confusing audit log entries, a potential attack vector, ...).
There was a problem hiding this comment.
OK, I think I understand.
Can you change the URL parameter name here from :fqdn to :untrustedFQDN or :userFQDN to hint that it's different from the return FQDN?
|
@andrejtokarcik could you create an issue for this PR? |
lib/web/apps.go
Outdated
| type CreateAppSessionRequest struct { | ||
| // FQDN is the fully qualified domain name of the application. | ||
| FQDN string `json:"fqdn"` | ||
| type AppResolveParams struct { |
There was a problem hiding this comment.
Let's rename this to AppResolveRequest for consistency.
There was a problem hiding this comment.
I'm not sure about this since these AppResolveParams are populated in the course of both getAppFQDN and createAppSession. Renaming this to AppResolveRequest would misleadingly indicate this is just the input part of a single request-response pair.
There was a problem hiding this comment.
Fair enough, please make wrapper types for individual endpoints, like:
type GetAppFQDNRequest AppResolveRequest(or ember a struct, whichever is cleaner)
| @@ -66,15 +66,58 @@ type CreateAppSessionRequest struct { | |||
| ClusterName string `json:"cluster_name"` | |||
| } | |||
|
|
|||
| type GetAppFQDNResponse struct { | |||
There was a problem hiding this comment.
AppResolveResponse for consistency?
lib/web/apps.go
Outdated
| type CreateAppSessionRequest struct { | ||
| // FQDN is the fully qualified domain name of the application. | ||
| FQDN string `json:"fqdn"` | ||
| type AppResolveParams struct { |
There was a problem hiding this comment.
Fair enough, please make wrapper types for individual endpoints, like:
type GetAppFQDNRequest AppResolveRequest(or ember a struct, whichever is cleaner)
andrejtokarcik
force-pushed
the
andrej/fix/double-CreateAppSession
branch
from
d913088 to
24c23a2
Compare
Currently, an app's target FQDN can be obtained only using the endpoint for creating new app sessions. The OAuth-style back-and-forth redirects between the app launcher and the app itself are therefore forced to generate an unnecessary additional app session just to resolve the FQDN. The new endpoint introduced here allows to resolve such FQDNs by invoking a dedicated endpoint.
Currently, an app's target FQDN can be obtained only using the endpoint
for creating new app sessions. The OAuth-style back-and-forth redirects
between the app launcher and the app itself are therefore forced to
generate an unnecessary additional app session just to resolve the FQDN.
The new endpoint introduced here allows to resolve such FQDNs by
invoking a dedicated endpoint.