diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index 9cfb4a81175ec..7f23469c8e211 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -77,9 +77,9 @@ import ( ) const ( - // ssoLoginConsoleErr is a generic error message to hide revealing sso login failure msgs. - ssoLoginConsoleErr = "Failed to login. Please check Teleport's log for more details." - metaRedirectHTML = ` + // SSOLoginFailureMessage is a generic error message to avoid disclosing sensitive SSO failure messages. + SSOLoginFailureMessage = "Failed to login. Please check Teleport's log for more details." + metaRedirectHTML = ` @@ -184,10 +184,10 @@ type Config struct { // Enables web UI if set. StaticFS http.FileSystem - // cachedSessionLingeringThreshold specifies the time the session will linger + // CachedSessionLingeringThreshold specifies the time the session will linger // in the cache before getting purged after it has expired. // Defaults to cachedSessionLingeringThreshold if unspecified. - cachedSessionLingeringThreshold *time.Duration + CachedSessionLingeringThreshold *time.Duration // ClusterFeatures contains flags for supported/unsupported features. ClusterFeatures proto.Features @@ -266,8 +266,8 @@ func NewHandler(cfg Config, opts ...HandlerOption) (*APIHandler, error) { } sessionLingeringThreshold := cachedSessionLingeringThreshold - if cfg.cachedSessionLingeringThreshold != nil { - sessionLingeringThreshold = *cfg.cachedSessionLingeringThreshold + if cfg.CachedSessionLingeringThreshold != nil { + sessionLingeringThreshold = *cfg.CachedSessionLingeringThreshold } auth, err := newSessionCache(sessionCacheOptions{ @@ -1211,17 +1211,17 @@ func (h *Handler) oidcLoginWeb(w http.ResponseWriter, r *http.Request, p httprou logger := h.log.WithField("auth", "oidc") logger.Debug("Web login start.") - req, err := parseSSORequestParams(r) + req, err := ParseSSORequestParams(r) if err != nil { logger.WithError(err).Error("Failed to extract SSO parameters from request.") return client.LoginFailedRedirectURL } response, err := h.cfg.ProxyClient.CreateOIDCAuthRequest(r.Context(), types.OIDCAuthRequest{ - CSRFToken: req.csrfToken, - ConnectorID: req.connectorID, + CSRFToken: req.CSRFToken, + ConnectorID: req.ConnectorID, CreateWebSession: true, - ClientRedirectURL: req.clientRedirectURL, + ClientRedirectURL: req.ClientRedirectURL, CheckUser: true, ProxyAddress: r.Host, }) @@ -1237,17 +1237,17 @@ func (h *Handler) githubLoginWeb(w http.ResponseWriter, r *http.Request, p httpr logger := h.log.WithField("auth", "github") logger.Debug("Web login start.") - req, err := parseSSORequestParams(r) + req, err := ParseSSORequestParams(r) if err != nil { logger.WithError(err).Error("Failed to extract SSO parameters from request.") return client.LoginFailedRedirectURL } response, err := h.cfg.ProxyClient.CreateGithubAuthRequest(r.Context(), types.GithubAuthRequest{ - CSRFToken: req.csrfToken, - ConnectorID: req.connectorID, + CSRFToken: req.CSRFToken, + ConnectorID: req.ConnectorID, CreateWebSession: true, - ClientRedirectURL: req.clientRedirectURL, + ClientRedirectURL: req.ClientRedirectURL, }) if err != nil { logger.WithError(err).Error("Error creating auth request.") @@ -1265,12 +1265,12 @@ func (h *Handler) githubLoginConsole(w http.ResponseWriter, r *http.Request, p h req := new(client.SSOLoginConsoleReq) if err := httplib.ReadJSON(r, req); err != nil { logger.WithError(err).Error("Error reading json.") - return nil, trace.AccessDenied(ssoLoginConsoleErr) + return nil, trace.AccessDenied(SSOLoginFailureMessage) } if err := req.CheckAndSetDefaults(); err != nil { logger.WithError(err).Error("Missing request parameters.") - return nil, trace.AccessDenied(ssoLoginConsoleErr) + return nil, trace.AccessDenied(SSOLoginFailureMessage) } response, err := h.cfg.ProxyClient.CreateGithubAuthRequest(r.Context(), types.GithubAuthRequest{ @@ -1285,7 +1285,7 @@ func (h *Handler) githubLoginConsole(w http.ResponseWriter, r *http.Request, p h }) if err != nil { logger.WithError(err).Error("Failed to create Github auth request.") - return nil, trace.AccessDenied(ssoLoginConsoleErr) + return nil, trace.AccessDenied(SSOLoginFailureMessage) } return &client.SSOLoginConsoleResponse{ @@ -1307,7 +1307,7 @@ func (h *Handler) githubCallback(w http.ResponseWriter, r *http.Request, p httpr // this improves the UX by terminating the failed SSO flow immediately, rather than hoping for a timeout. if requestID := r.URL.Query().Get("state"); requestID != "" { if request, errGet := h.cfg.ProxyClient.GetGithubAuthRequest(r.Context(), requestID); errGet == nil && !request.CreateWebSession { - if redURL, errEnc := redirectURLWithError(request.ClientRedirectURL, err); errEnc == nil { + if redURL, errEnc := RedirectURLWithError(request.ClientRedirectURL, err); errEnc == nil { return redURL.String() } } @@ -1323,19 +1323,19 @@ func (h *Handler) githubCallback(w http.ResponseWriter, r *http.Request, p httpr if response.Req.CreateWebSession { logger.Infof("Redirecting to web browser.") - res := &ssoCallbackResponse{ - csrfToken: response.Req.CSRFToken, - username: response.Username, - sessionName: response.Session.GetName(), - clientRedirectURL: response.Req.ClientRedirectURL, + res := &SSOCallbackResponse{ + CSRFToken: response.Req.CSRFToken, + Username: response.Username, + SessionName: response.Session.GetName(), + ClientRedirectURL: response.Req.ClientRedirectURL, } - if err := ssoSetWebSessionAndRedirectURL(w, r, res, true); err != nil { + if err := SSOSetWebSessionAndRedirectURL(w, r, res, true); err != nil { logger.WithError(err).Error("Error setting web session.") return client.LoginFailedRedirectURL } - return res.clientRedirectURL + return res.ClientRedirectURL } logger.Infof("Callback is redirecting to console login.") @@ -1369,12 +1369,12 @@ func (h *Handler) oidcLoginConsole(w http.ResponseWriter, r *http.Request, p htt req := new(client.SSOLoginConsoleReq) if err := httplib.ReadJSON(r, req); err != nil { logger.WithError(err).Error("Error reading json.") - return nil, trace.AccessDenied(ssoLoginConsoleErr) + return nil, trace.AccessDenied(SSOLoginFailureMessage) } if err := req.CheckAndSetDefaults(); err != nil { logger.WithError(err).Error("Missing request parameters.") - return nil, trace.AccessDenied(ssoLoginConsoleErr) + return nil, trace.AccessDenied(SSOLoginFailureMessage) } response, err := h.cfg.ProxyClient.CreateOIDCAuthRequest(r.Context(), types.OIDCAuthRequest{ @@ -1391,7 +1391,7 @@ func (h *Handler) oidcLoginConsole(w http.ResponseWriter, r *http.Request, p htt }) if err != nil { logger.WithError(err).Error("Failed to create OIDC auth request.") - return nil, trace.AccessDenied(ssoLoginConsoleErr) + return nil, trace.AccessDenied(SSOLoginFailureMessage) } return &client.SSOLoginConsoleResponse{ @@ -1413,7 +1413,7 @@ func (h *Handler) oidcCallback(w http.ResponseWriter, r *http.Request, p httprou // this improves the UX by terminating the failed SSO flow immediately, rather than hoping for a timeout. if requestID := r.URL.Query().Get("state"); requestID != "" { if request, errGet := h.cfg.ProxyClient.GetOIDCAuthRequest(r.Context(), requestID); errGet == nil && !request.CreateWebSession { - if redURL, errEnc := redirectURLWithError(request.ClientRedirectURL, err); errEnc == nil { + if redURL, errEnc := RedirectURLWithError(request.ClientRedirectURL, err); errEnc == nil { return redURL.String() } } @@ -1430,19 +1430,19 @@ func (h *Handler) oidcCallback(w http.ResponseWriter, r *http.Request, p httprou if response.Req.CreateWebSession { logger.Info("Redirecting to web browser.") - res := &ssoCallbackResponse{ - csrfToken: response.Req.CSRFToken, - username: response.Username, - sessionName: response.Session.GetName(), - clientRedirectURL: response.Req.ClientRedirectURL, + res := &SSOCallbackResponse{ + CSRFToken: response.Req.CSRFToken, + Username: response.Username, + SessionName: response.Session.GetName(), + ClientRedirectURL: response.Req.ClientRedirectURL, } - if err := ssoSetWebSessionAndRedirectURL(w, r, res, true); err != nil { + if err := SSOSetWebSessionAndRedirectURL(w, r, res, true); err != nil { logger.WithError(err).Error("Error setting web session.") return client.LoginFailedRedirectURL } - return res.clientRedirectURL + return res.ClientRedirectURL } logger.Info("Callback redirecting to console login.") @@ -1593,7 +1593,11 @@ func ConstructSSHResponse(response AuthParams) (*url.URL, error) { return u, nil } -func redirectURLWithError(clientRedirectURL string, errReply error) (*url.URL, error) { +// RedirectURLWithError adds an err query parameter to the given redirect URL with the +// given errReply message and returns the new URL. If the given URL cannot be parsed, +// an error is returned with a nil URL. It is used to return an error back to the +// original URL in an SSO callback when validation fails. +func RedirectURLWithError(clientRedirectURL string, errReply error) (*url.URL, error) { u, err := url.Parse(clientRedirectURL) if err != nil { return nil, trace.Wrap(err) @@ -3167,13 +3171,23 @@ func makeTeleportClientConfig(ctx context.Context, sesCtx *SessionContext) (*cli return config, nil } -type ssoRequestParams struct { - clientRedirectURL string - connectorID string - csrfToken string +// SSORequestParams holds parameters parsed out of a HTTP request initiating an +// SSO login. See ParseSSORequestParams(). +type SSORequestParams struct { + // ClientRedirectURL is the URL specified in the query parameter + // redirect_url, which will be unescaped here. + ClientRedirectURL string + // ConnectorID identifies the SSO connector to use to log in, from + // the connector_id query parameter. + ConnectorID string + // CSRFToken is the token in the CSRF cookie header. + CSRFToken string } -func parseSSORequestParams(r *http.Request) (*ssoRequestParams, error) { +// ParseSSORequestParams extracts the SSO request parameters from an http.Request, +// returning them in an SSORequestParams struct. If any fields are not present, +// an error is returned. +func ParseSSORequestParams(r *http.Request) (*SSORequestParams, error) { // Manually grab the value from query param "redirect_url". // // The "redirect_url" param can contain its own query params such as in @@ -3205,37 +3219,52 @@ func parseSSORequestParams(r *http.Request) (*ssoRequestParams, error) { return nil, trace.Wrap(err) } - return &ssoRequestParams{ - clientRedirectURL: clientRedirectURL, - connectorID: connectorID, - csrfToken: csrfToken, + return &SSORequestParams{ + ClientRedirectURL: clientRedirectURL, + ConnectorID: connectorID, + CSRFToken: csrfToken, }, nil } -type ssoCallbackResponse struct { - csrfToken string - username string - sessionName string - clientRedirectURL string +// SSOCallbackResponse holds the parameters for validating and executing an SSO +// callback URL. See SSOSetWebSessionAndRedirectURL(). +type SSOCallbackResponse struct { + // CSRFToken is the token provided in the originating SSO login request + // to be validated against. + CSRFToken string + // Username is the authenticated teleport username of the user that has + // logged in, provided by the SSO provider. + Username string + // SessionName is the name of the session generated by auth server if + // requested in the SSO request. + SessionName string + // ClientRedirectURL is the URL to redirect back to on completion of + // the SSO login process. + ClientRedirectURL string } -func ssoSetWebSessionAndRedirectURL(w http.ResponseWriter, r *http.Request, response *ssoCallbackResponse, verifyCSRF bool) error { +// SSOSetWebSessionAndRedirectURL validates the CSRF token in the response +// against that in the request, validates that the callback URL in the response +// can be parsed, and sets a session cookie with the username and session name +// from the response. On success, nil is returned. If the validation fails, an +// error is returned. +func SSOSetWebSessionAndRedirectURL(w http.ResponseWriter, r *http.Request, response *SSOCallbackResponse, verifyCSRF bool) error { if verifyCSRF { - if err := csrf.VerifyToken(response.csrfToken, r); err != nil { + if err := csrf.VerifyToken(response.CSRFToken, r); err != nil { return trace.Wrap(err) } } - if err := SetSessionCookie(w, response.username, response.sessionName); err != nil { + if err := SetSessionCookie(w, response.Username, response.SessionName); err != nil { return trace.Wrap(err) } - parsedURL, err := url.Parse(response.clientRedirectURL) + parsedURL, err := url.Parse(response.ClientRedirectURL) if err != nil { return trace.Wrap(err) } - response.clientRedirectURL = parsedURL.RequestURI() + response.ClientRedirectURL = parsedURL.RequestURI() return nil } diff --git a/lib/web/apiserver_test.go b/lib/web/apiserver_test.go index 3e14d583c25d3..fd8a0ef86f002 100644 --- a/lib/web/apiserver_test.go +++ b/lib/web/apiserver_test.go @@ -359,7 +359,7 @@ func newWebSuite(t *testing.T) *WebSuite { HostUUID: proxyID, Emitter: s.proxyClient, StaticFS: fs, - cachedSessionLingeringThreshold: &sessionLingeringThreshold, + CachedSessionLingeringThreshold: &sessionLingeringThreshold, ProxySettings: &mockProxySettings{}, }, SetSessionStreamPollPeriod(200*time.Millisecond), SetClock(s.clock)) require.NoError(t, err) @@ -3712,33 +3712,33 @@ func TestParseSSORequestParams(t *testing.T) { tests := []struct { name, url string wantErr bool - expected *ssoRequestParams + expected *SSORequestParams }{ { name: "preserve redirect's query params (escaped)", url: "https://localhost/login?connector_id=oidc&redirect_url=https:%2F%2Flocalhost:8080%2Fweb%2Fcluster%2Fim-a-cluster-name%2Fnodes%3Fsearch=tunnel&sort=hostname:asc", - expected: &ssoRequestParams{ - clientRedirectURL: "https://localhost:8080/web/cluster/im-a-cluster-name/nodes?search=tunnel&sort=hostname:asc", - connectorID: "oidc", - csrfToken: token, + expected: &SSORequestParams{ + ClientRedirectURL: "https://localhost:8080/web/cluster/im-a-cluster-name/nodes?search=tunnel&sort=hostname:asc", + ConnectorID: "oidc", + CSRFToken: token, }, }, { name: "preserve redirect's query params (unescaped)", url: "https://localhost/login?connector_id=github&redirect_url=https://localhost:8080/web/cluster/im-a-cluster-name/nodes?search=tunnel&sort=hostname:asc", - expected: &ssoRequestParams{ - clientRedirectURL: "https://localhost:8080/web/cluster/im-a-cluster-name/nodes?search=tunnel&sort=hostname:asc", - connectorID: "github", - csrfToken: token, + expected: &SSORequestParams{ + ClientRedirectURL: "https://localhost:8080/web/cluster/im-a-cluster-name/nodes?search=tunnel&sort=hostname:asc", + ConnectorID: "github", + CSRFToken: token, }, }, { name: "preserve various encoded chars", url: "https://localhost/login?connector_id=saml&redirect_url=https:%2F%2Flocalhost:8080%2Fweb%2Fcluster%2Fim-a-cluster-name%2Fapps%3Fquery=search(%2522watermelon%2522%252C%2520%2522this%2522)%2520%2526%2526%2520labels%255B%2522unique-id%2522%255D%2520%253D%253D%2520%2522hi%2522&sort=name:asc", - expected: &ssoRequestParams{ - clientRedirectURL: "https://localhost:8080/web/cluster/im-a-cluster-name/apps?query=search(%22watermelon%22%2C%20%22this%22)%20%26%26%20labels%5B%22unique-id%22%5D%20%3D%3D%20%22hi%22&sort=name:asc", - connectorID: "saml", - csrfToken: token, + expected: &SSORequestParams{ + ClientRedirectURL: "https://localhost:8080/web/cluster/im-a-cluster-name/apps?query=search(%22watermelon%22%2C%20%22this%22)%20%26%26%20labels%5B%22unique-id%22%5D%20%3D%3D%20%22hi%22&sort=name:asc", + ConnectorID: "saml", + CSRFToken: token, }, }, { @@ -3759,7 +3759,7 @@ func TestParseSSORequestParams(t *testing.T) { require.NoError(t, err) addCSRFCookieToReq(req, token) - params, err := parseSSORequestParams(req) + params, err := ParseSSORequestParams(req) switch { case tc.wantErr: diff --git a/lib/web/saml.go b/lib/web/saml.go index f16fb08ce3168..9f17c517fa3c1 100644 --- a/lib/web/saml.go +++ b/lib/web/saml.go @@ -35,17 +35,17 @@ func (h *Handler) samlSSO(w http.ResponseWriter, r *http.Request, p httprouter.P logger := h.log.WithField("auth", "saml") logger.Debug("Web login start.") - req, err := parseSSORequestParams(r) + req, err := ParseSSORequestParams(r) if err != nil { logger.WithError(err).Error("Failed to extract SSO parameters from request.") return client.LoginFailedRedirectURL } response, err := h.cfg.ProxyClient.CreateSAMLAuthRequest(r.Context(), types.SAMLAuthRequest{ - ConnectorID: req.connectorID, - CSRFToken: req.csrfToken, + ConnectorID: req.ConnectorID, + CSRFToken: req.CSRFToken, CreateWebSession: true, - ClientRedirectURL: req.clientRedirectURL, + ClientRedirectURL: req.ClientRedirectURL, }) if err != nil { logger.WithError(err).Error("Error creating auth request.") @@ -62,12 +62,12 @@ func (h *Handler) samlSSOConsole(w http.ResponseWriter, r *http.Request, p httpr req := new(client.SSOLoginConsoleReq) if err := httplib.ReadJSON(r, req); err != nil { logger.WithError(err).Error("Error reading json.") - return nil, trace.AccessDenied(ssoLoginConsoleErr) + return nil, trace.AccessDenied(SSOLoginFailureMessage) } if err := req.CheckAndSetDefaults(); err != nil { logger.WithError(err).Error("Missing request parameters.") - return nil, trace.AccessDenied(ssoLoginConsoleErr) + return nil, trace.AccessDenied(SSOLoginFailureMessage) } response, err := h.cfg.ProxyClient.CreateSAMLAuthRequest(r.Context(), types.SAMLAuthRequest{ @@ -82,7 +82,7 @@ func (h *Handler) samlSSOConsole(w http.ResponseWriter, r *http.Request, p httpr }) if err != nil { logger.WithError(err).Error("Failed to create SAML auth request.") - return nil, trace.AccessDenied(ssoLoginConsoleErr) + return nil, trace.AccessDenied(SSOLoginFailureMessage) } return &client.SSOLoginConsoleResponse{RedirectURL: response.RedirectURL}, nil @@ -109,7 +109,7 @@ func (h *Handler) samlACS(w http.ResponseWriter, r *http.Request, p httprouter.P // this improves the UX by terminating the failed SSO flow immediately, rather than hoping for a timeout. if requestID, errParse := auth.ParseSAMLInResponseTo(samlResponse); errParse == nil { if request, errGet := h.cfg.ProxyClient.GetSAMLAuthRequest(r.Context(), requestID); errGet == nil && !request.CreateWebSession { - if url, errEnc := redirectURLWithError(request.ClientRedirectURL, err); errEnc == nil { + if url, errEnc := RedirectURLWithError(request.ClientRedirectURL, err); errEnc == nil { return url.String() } } @@ -131,19 +131,19 @@ func (h *Handler) samlACS(w http.ResponseWriter, r *http.Request, p httprouter.P redirect = "/web/" } - res := &ssoCallbackResponse{ - csrfToken: response.Req.CSRFToken, - username: response.Username, - sessionName: response.Session.GetName(), - clientRedirectURL: redirect, + res := &SSOCallbackResponse{ + CSRFToken: response.Req.CSRFToken, + Username: response.Username, + SessionName: response.Session.GetName(), + ClientRedirectURL: redirect, } - if err := ssoSetWebSessionAndRedirectURL(w, r, res, response.Req.CSRFToken != ""); err != nil { + if err := SSOSetWebSessionAndRedirectURL(w, r, res, response.Req.CSRFToken != ""); err != nil { logger.WithError(err).Error("Error setting web session.") return client.LoginFailedRedirectURL } - return res.clientRedirectURL + return res.ClientRedirectURL } logger.Debug("Callback redirecting to console login.")