diff --git a/integration/appaccess/appaccess_test.go b/integration/appaccess/appaccess_test.go index 94afd18cbf99b..612e08c4563f5 100644 --- a/integration/appaccess/appaccess_test.go +++ b/integration/appaccess/appaccess_test.go @@ -119,9 +119,11 @@ func testForward(p *Pack, t *testing.T) { }, { desc: "invalid application session cookie, redirect to login", - inCookies: []*http.Cookie{{ - Name: app.CookieName, - Value: "D25C463CD27861559CC6A0A6AE54818079809AA8731CB18037B4B37A80C4FC6C"}, + inCookies: []*http.Cookie{ + { + Name: app.CookieName, + Value: "D25C463CD27861559CC6A0A6AE54818079809AA8731CB18037B4B37A80C4FC6C", + }, }, outStatusCode: http.StatusFound, }, @@ -576,7 +578,7 @@ func testInvalidateAppSessionsOnLogout(p *Pack, t *testing.T) { require.Equal(t, http.StatusOK, status) // Logout from Teleport. - status, _, err = p.makeWebapiRequest(http.MethodDelete, "sessions", []byte{}) + status, _, err = p.makeWebapiRequest(http.MethodDelete, "sessions/web", []byte{}) require.NoError(t, err) require.Equal(t, http.StatusOK, status) diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index 3a948af7a6d3c..0a6400db3701c 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -112,6 +112,7 @@ type Handler struct { auth *sessionCache sessionStreamPollPeriod time.Duration clock clockwork.Clock + limiter *limiter.RateLimiter // sshPort specifies the SSH proxy port extracted // from configuration sshPort string @@ -315,7 +316,7 @@ func NewHandler(cfg Config, opts ...HandlerOption) (*APIHandler, error) { // rateLimiter is used to limit unauthenticated challenge generation for // passwordless and for unauthenticated metrics. - rateLimiter, err := limiter.NewRateLimiter(limiter.Config{ + h.limiter, err = limiter.NewRateLimiter(limiter.Config{ Rates: []limiter.Rate{ { Period: defaults.LimiterPasswordlessPeriod, @@ -333,7 +334,7 @@ func NewHandler(cfg Config, opts ...HandlerOption) (*APIHandler, error) { if cfg.MinimalReverseTunnelRoutesOnly { h.bindMinimalEndpoints() } else { - h.bindDefaultEndpoints(rateLimiter) + h.bindDefaultEndpoints() } // if Web UI is enabled, check the assets dir: @@ -472,7 +473,7 @@ func (h *Handler) bindMinimalEndpoints() { } // bindDefaultEndpoints binds the default endpoints for the web API. -func (h *Handler) bindDefaultEndpoints(challengeLimiter *limiter.RateLimiter) { +func (h *Handler) bindDefaultEndpoints() { h.bindMinimalEndpoints() // ping endpoint is used to check if the server is up. the /webapi/ping @@ -497,20 +498,22 @@ func (h *Handler) bindDefaultEndpoints(challengeLimiter *limiter.RateLimiter) { h.GET("/webapi/scripts/desktop-access/install-ad-cs.ps1", httplib.MakeHandler(h.desktopAccessScriptInstallADCSHandle)) h.GET("/webapi/scripts/desktop-access/configure/:token/configure-ad.ps1", httplib.MakeHandler(h.desktopAccessScriptConfigureHandle)) - // DELETE IN: 5.1.0 - // - // Migrated this endpoint to /webapi/sessions/web below. - h.POST("/webapi/sessions", httplib.WithCSRFProtection(h.createWebSession)) - // Forwards traces to the configured upstream collector h.POST("/webapi/traces", h.WithAuth(h.traces)) - // Web sessions - h.POST("/webapi/sessions/web", httplib.WithCSRFProtection(h.createWebSession)) + // App sessions h.POST("/webapi/sessions/app", h.WithAuth(h.createAppSession)) - h.DELETE("/webapi/sessions", h.WithAuth(h.deleteSession)) - h.POST("/webapi/sessions/renew", h.WithAuth(h.renewSession)) + // DELETE IN 13, deprecated/unused web sessions routes (avatus) + // https://github.com/gravitational/teleport/pull/19892 + h.POST("/webapi/sessions", httplib.WithCSRFProtection(h.WithLimiterHandlerFunc(h.createWebSession))) + h.DELETE("/webapi/sessions", h.WithAuth(h.deleteWebSession)) + h.POST("/webapi/sessions/renew", h.WithAuth(h.renewWebSession)) + + // Web sessions + h.POST("/webapi/sessions/web", httplib.WithCSRFProtection(h.WithLimiterHandlerFunc(h.createWebSession))) + h.DELETE("/webapi/sessions/web", h.WithAuth(h.deleteWebSession)) + h.POST("/webapi/sessions/web/renew", h.WithAuth(h.renewWebSession)) h.POST("/webapi/users", h.WithAuth(h.createUserHandle)) h.PUT("/webapi/users", h.WithAuth(h.updateUserHandle)) h.GET("/webapi/users", h.WithAuth(h.getUsersHandle)) @@ -609,10 +612,10 @@ func (h *Handler) bindDefaultEndpoints(challengeLimiter *limiter.RateLimiter) { // Github connector handlers h.GET("/webapi/github/login/web", h.WithRedirect(h.githubLoginWeb)) h.GET("/webapi/github/callback", h.WithMetaRedirect(h.githubCallback)) - h.POST("/webapi/github/login/console", httplib.MakeHandler(h.githubLoginConsole)) + h.POST("/webapi/github/login/console", h.WithLimiter(h.githubLoginConsole)) // MFA public endpoints. - h.POST("/webapi/mfa/login/begin", h.withLimiter(challengeLimiter, h.mfaLoginBegin)) + h.POST("/webapi/mfa/login/begin", h.WithLimiter(h.mfaLoginBegin)) h.POST("/webapi/mfa/login/finish", httplib.MakeHandler(h.mfaLoginFinish)) h.POST("/webapi/mfa/login/finishsession", httplib.MakeHandler(h.mfaLoginFinishSession)) h.DELETE("/webapi/mfa/token/:token/devices/:devicename", httplib.MakeHandler(h.deleteMFADeviceWithTokenHandle)) @@ -669,7 +672,7 @@ func (h *Handler) bindDefaultEndpoints(challengeLimiter *limiter.RateLimiter) { h.GET("/webapi/connectionupgrade", httplib.MakeHandler(h.connectionUpgrade)) // create user events. - h.POST("/webapi/precapture", h.withLimiter(challengeLimiter, h.createPreUserEventHandle)) + h.POST("/webapi/precapture", h.WithLimiter(h.createPreUserEventHandle)) // create authenticated user events. h.POST("/webapi/capture", h.WithAuth(h.createUserEventHandle)) } @@ -1856,14 +1859,14 @@ func clientMetaFromReq(r *http.Request) *auth.ForwardedClientMetadata { } } -// deleteSession is called to sign out user +// deleteWebSession is called to sign out user // // DELETE /v1/webapi/sessions/:sid // // Response: // // {"message": "ok"} -func (h *Handler) deleteSession(w http.ResponseWriter, r *http.Request, _ httprouter.Params, ctx *SessionContext) (interface{}, error) { +func (h *Handler) deleteWebSession(w http.ResponseWriter, r *http.Request, _ httprouter.Params, ctx *SessionContext) (interface{}, error) { err := h.logout(r.Context(), w, ctx) if err != nil { return nil, trace.Wrap(err) @@ -1891,14 +1894,14 @@ type renewSessionRequest struct { ReloadUser bool `json:"reloadUser"` } -// renewSession updates this existing session with a new session. +// renewWebSession updates this existing session with a new session. // // Depending on request fields sent in for extension, the new session creation can vary depending on: // - AccessRequestID (opt): appends roles approved from access request to currently assigned roles or, // - Switchback (opt): roles stacked with assuming approved access requests, will revert to user's default roles // - ReloadUser (opt): similar to default but updates user related data (e.g login traits) by retrieving it from the backend // - default (none set): create new session with currently assigned roles -func (h *Handler) renewSession(w http.ResponseWriter, r *http.Request, params httprouter.Params, ctx *SessionContext) (interface{}, error) { +func (h *Handler) renewWebSession(w http.ResponseWriter, r *http.Request, params httprouter.Params, ctx *SessionContext) (interface{}, error) { req := renewSessionRequest{} if err := httplib.ReadJSON(r, &req); err != nil { return nil, trace.Wrap(err) @@ -3404,10 +3407,22 @@ func (h *Handler) WithAuth(fn ContextHandler) httprouter.Handle { }) } -// withLimiter adds IP-based rate limiting to fn. -func (h *Handler) withLimiter(l *limiter.RateLimiter, fn httplib.HandlerFunc) httprouter.Handle { +// WithLimiter adds IP-based rate limiting to fn. +func (h *Handler) WithLimiter(fn httplib.HandlerFunc) httprouter.Handle { return httplib.MakeHandler(func(w http.ResponseWriter, r *http.Request, p httprouter.Params) (interface{}, error) { - err := l.RegisterRequest(r.RemoteAddr, nil /* customRate */) + return h.WithLimiterHandlerFunc(fn)(w, r, p) + }) +} + +// WithLimiterHandlerFunc adds IP-based rate limiting to a HandlerFunc. This +// should be used when you need to nest this inside another HandlerFunc. +func (h *Handler) WithLimiterHandlerFunc(fn httplib.HandlerFunc) httplib.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request, p httprouter.Params) (interface{}, error) { + remote, _, err := net.SplitHostPort(r.RemoteAddr) + if err != nil { + return nil, trace.Wrap(err) + } + err = h.limiter.RegisterRequest(remote, nil /* customRate */) // MaxRateError doesn't play well with errors.Is, hence the cast. if _, ok := err.(*ratelimit.MaxRateError); ok { return nil, trace.LimitExceeded(err.Error()) @@ -3416,7 +3431,7 @@ func (h *Handler) withLimiter(l *limiter.RateLimiter, fn httplib.HandlerFunc) ht return nil, trace.Wrap(err) } return fn(w, r, p) - }) + } } // AuthenticateRequest authenticates request using combination of a session cookie diff --git a/lib/web/apiserver_test.go b/lib/web/apiserver_test.go index fd382df2af652..f7e12d283389a 100644 --- a/lib/web/apiserver_test.go +++ b/lib/web/apiserver_test.go @@ -58,6 +58,7 @@ import ( "github.com/jonboulle/clockwork" "github.com/julienschmidt/httprouter" lemma_secret "github.com/mailgun/lemma/secret" + "github.com/mailgun/timetools" "github.com/pquerna/otp/totp" "github.com/sirupsen/logrus" "github.com/stretchr/testify/require" @@ -579,7 +580,7 @@ func noCache(clt auth.ClientI, cacheName []string) (auth.RemoteProxyAccessPoint, } func (r *authPack) renewSession(ctx context.Context, t *testing.T) *roundtrip.Response { - resp, err := r.clt.PostJSON(ctx, r.clt.Endpoint("webapi", "sessions", "renew"), nil) + resp, err := r.clt.PostJSON(ctx, r.clt.Endpoint("webapi", "sessions", "web", "renew"), nil) require.NoError(t, err) return resp } @@ -877,7 +878,7 @@ func TestWebSessionsCRUD(t *testing.T) { // now delete session _, err = pack.clt.Delete( context.Background(), - pack.clt.Endpoint("webapi", "sessions")) + pack.clt.Endpoint("webapi", "sessions", "web")) require.NoError(t, err) // subsequent requests trying to use this session will fail @@ -1029,7 +1030,7 @@ func TestWebSessionsBadInput(t *testing.T) { } for i, req := range reqs { t.Run(fmt.Sprintf("tc %v", i), func(t *testing.T) { - _, err := clt.PostJSON(s.ctx, clt.Endpoint("webapi", "sessions"), req) + _, err := clt.PostJSON(s.ctx, clt.Endpoint("webapi", "sessions", "web"), req) require.Error(t, err) require.True(t, trace.IsAccessDenied(err)) }) @@ -1229,7 +1230,8 @@ func TestNewTerminalHandler(t *testing.T) { ID: session.ID("not a uuid"), }, }, - }, { + }, + { expectedErr: "login: missing login", cfg: TerminalHandlerConfig{ SessionData: session.Session{ @@ -1237,7 +1239,8 @@ func TestNewTerminalHandler(t *testing.T) { Login: "", }, }, - }, { + }, + { expectedErr: "server: missing server", cfg: TerminalHandlerConfig{ SessionData: session.Session{ @@ -2021,7 +2024,7 @@ func TestCloseConnectionsOnLogout(t *testing.T) { _, err = stream.Read(out) require.NoError(t, err) - _, err = pack.clt.Delete(s.ctx, pack.clt.Endpoint("webapi", "sessions")) + _, err = pack.clt.Delete(s.ctx, pack.clt.Endpoint("webapi", "sessions", "web")) require.NoError(t, err) // wait until we timeout or detect that connection has been closed @@ -2131,7 +2134,7 @@ func TestLogin_PrivateKeyEnabledError(t *testing.T) { require.NoError(t, err) clt := s.client() - req, err := http.NewRequest("POST", clt.Endpoint("webapi", "sessions"), bytes.NewBuffer(loginReq)) + req, err := http.NewRequest("POST", clt.Endpoint("webapi", "sessions", "web"), bytes.NewBuffer(loginReq)) require.NoError(t, err) ua := "test-ua" req.Header.Set("User-Agent", ua) @@ -2171,7 +2174,7 @@ func TestLogin(t *testing.T) { clt := s.client() ua := "test-ua" - req, err := http.NewRequest("POST", clt.Endpoint("webapi", "sessions"), bytes.NewBuffer(loginReq)) + req, err := http.NewRequest("POST", clt.Endpoint("webapi", "sessions", "web"), bytes.NewBuffer(loginReq)) require.NoError(t, err) req.Header.Set("User-Agent", ua) @@ -3759,7 +3762,7 @@ func TestApplicationWebSessionsDeletedAfterLogout(t *testing.T) { require.Len(t, sessions, len(applications)) // Logout from Telport. - _, err = pack.clt.Delete(context.Background(), pack.clt.Endpoint("webapi", "sessions")) + _, err = pack.clt.Delete(context.Background(), pack.clt.Endpoint("webapi", "sessions", "web")) require.NoError(t, err) // Check sessions after logout, should be empty. @@ -4505,7 +4508,7 @@ func TestWebSessionsRenewAllowsOldBearerTokenToLinger(t *testing.T) { // now delete session _, err = newPack.clt.Delete( context.Background(), - pack.clt.Endpoint("webapi", "sessions")) + pack.clt.Endpoint("webapi", "sessions", "web")) require.NoError(t, err) // subsequent requests to use this session will fail @@ -6174,7 +6177,7 @@ func (s *WebSuite) login(clt *client.WebClient, cookieToken string, reqToken str if err != nil { return nil, err } - req, err := http.NewRequest("POST", clt.Endpoint("webapi", "sessions"), bytes.NewBuffer(data)) + req, err := http.NewRequest("POST", clt.Endpoint("webapi", "sessions", "web"), bytes.NewBuffer(data)) if err != nil { return nil, err } @@ -6802,7 +6805,7 @@ func login(t *testing.T, clt *client.WebClient, cookieToken, reqToken string, re if err != nil { return nil, err } - req, err := http.NewRequest("POST", clt.Endpoint("webapi", "sessions"), bytes.NewBuffer(data)) + req, err := http.NewRequest("POST", clt.Endpoint("webapi", "sessions", "web"), bytes.NewBuffer(data)) if err != nil { return nil, err } @@ -6880,7 +6883,7 @@ func TestUserContextWithAccessRequest(t *testing.T) { accessRequestID := accessReq.GetMetadata().Name // Make a request to renew the session with the ID of the access request. - _, err = pack.clt.PostJSON(ctx, pack.clt.Endpoint("webapi", "sessions", "renew"), renewSessionRequest{ + _, err = pack.clt.PostJSON(ctx, pack.clt.Endpoint("webapi", "sessions", "web", "renew"), renewSessionRequest{ AccessRequestID: accessRequestID, }) require.NoError(t, err) @@ -6899,6 +6902,40 @@ func TestUserContextWithAccessRequest(t *testing.T) { require.Equal(t, accessRequestID, userContext.ConsumedAccessRequestID) } +func TestWithLimiterHandlerFunc(t *testing.T) { + const burst = 20 + limiter, err := limiter.NewRateLimiter(limiter.Config{ + Rates: []limiter.Rate{ + { + Period: time.Minute, + Average: 10, + Burst: burst, + }, + }, + Clock: &timetools.FreezedTime{ + CurrentTime: time.Date(2016, 6, 5, 4, 3, 2, 1, time.UTC), + }, + }) + require.NoError(t, err) + h := &Handler{limiter: limiter} + hf := h.WithLimiterHandlerFunc(func(http.ResponseWriter, *http.Request, httprouter.Params) (interface{}, error) { + return nil, nil + }) + + // Verify that a valid burst is allowed. + r := &http.Request{} + for i := 0; i < burst; i++ { + r.RemoteAddr = fmt.Sprintf("127.0.0.1:%v", i) + _, err = hf(nil, r, nil) + require.NoError(t, err, "WithLimiterHandlerFunc failed unexpectedly") + } + + // Verify that exceeding the limit causes errors. + r.RemoteAddr = fmt.Sprintf("127.0.0.1:%v", burst) + _, err = hf(nil, r, nil) + require.True(t, trace.IsLimitExceeded(err), "WithLimiterHandlerFunc returned err = %T, want trace.LimitExceededError", err) +} + // kubeClusterConfig defines the cluster to be created type kubeClusterConfig struct { name string @@ -7374,7 +7411,7 @@ func TestLogout(t *testing.T) { require.Len(t, clusters, 1) // logout from proxy 1 - _, err = pack.clt.Delete(ctx, pack.clt.Endpoint("webapi", "sessions")) + _, err = pack.clt.Delete(ctx, pack.clt.Endpoint("webapi", "sessions", "web")) require.NoError(t, err) // ensure proxy 1 invalidated the session