From f556046b126b7325ced1b66aae1920527b0108dd Mon Sep 17 00:00:00 2001 From: Alan Parra Date: Mon, 4 Mar 2024 17:56:09 -0300 Subject: [PATCH] [v13] Move lib/web tests to new web login helpers (#38890) * Move lib/web tests to new web login helpers (#38806) * Move login helpers to a separate file * Reuse newOTPSharedSecret * Add login method variants and tweak signatures * Fix TestWebSessionsBadInput * Move tests to new web login helpers * Unify and simplify helpers, verify response status * Fix TestPasswordChange * Drop unused functions from backport --- lib/web/apiserver_test.go | 328 ++++++++++++++++------------------- lib/web/login_helper_test.go | 158 +++++++++++++++++ 2 files changed, 304 insertions(+), 182 deletions(-) create mode 100644 lib/web/login_helper_test.go diff --git a/lib/web/apiserver_test.go b/lib/web/apiserver_test.go index bd131a4dbd0ae..2119b12ad3cd6 100644 --- a/lib/web/apiserver_test.go +++ b/lib/web/apiserver_test.go @@ -25,7 +25,6 @@ import ( "crypto" "crypto/tls" "crypto/x509" - "encoding/base32" "encoding/base64" "encoding/hex" "encoding/json" @@ -631,9 +630,8 @@ type authPack struct { // user, otp token, created web session and authenticated client. func (s *WebSuite) authPack(t *testing.T, user string, roles ...string) *authPack { login := s.user - pass := "abc123" - rawSecret := "def456" - otpSecret := base32.StdEncoding.EncodeToString([]byte(rawSecret)) + pass := "abcdef123456" + otpSecret := newOTPSharedSecret() ap, err := types.NewAuthPreference(types.AuthPreferenceSpecV2{ Type: constants.Local, @@ -645,40 +643,32 @@ func (s *WebSuite) authPack(t *testing.T, user string, roles ...string) *authPac s.createUser(t, user, login, pass, otpSecret, roles...) - // create a valid otp token - validToken, err := totp.GenerateCode(otpSecret, s.clock.Now()) - require.NoError(t, err) - - clt := s.client(t) - req := CreateSessionReq{ - User: user, - Pass: pass, - SecondFactorToken: validToken, - } - - csrfToken := "2ebcb768d0090ea4368e42880c970b61865c326172a4a2343b645cf5d7f20992" - re, err := s.login(clt, csrfToken, csrfToken, req) - require.NoError(t, err) - - var rawSess *CreateSessionResponse - require.NoError(t, json.Unmarshal(re.Bytes(), &rawSess)) + ctx := context.Background() + sessionResp, httpResp := loginWebOTP(t, ctx, loginWebOTPParams{ + webClient: s.client(t), + clock: s.clock, + user: user, + password: pass, + otpSecret: otpSecret, + }) - sess, err := rawSess.response() + sess, err := sessionResp.response() require.NoError(t, err) jar, err := cookiejar.New(nil) require.NoError(t, err) - clt = s.client(t, roundtrip.BearerAuth(sess.Token), roundtrip.CookieJar(jar)) - jar.SetCookies(s.url(), re.Cookies()) + clt := s.client(t, roundtrip.BearerAuth(sess.Token), roundtrip.CookieJar(jar)) + jar.SetCookies(s.url(), httpResp.Cookies()) return &authPack{ otpSecret: otpSecret, user: user, login: login, + password: pass, session: sess, clt: clt, - cookies: re.Cookies(), + cookies: httpResp.Cookies(), } } @@ -913,19 +903,10 @@ func TestCSRF(t *testing.T) { // create a valid user user := "csrfuser" - pass := "abc123" - otpSecret := base32.StdEncoding.EncodeToString([]byte("def456")) + pass := "abcdef123456" + otpSecret := newOTPSharedSecret() s.createUser(t, user, user, pass, otpSecret) - // create a valid login form request - validToken, err := totp.GenerateCode(otpSecret, time.Now()) - require.NoError(t, err) - loginForm := CreateSessionReq{ - User: user, - Pass: pass, - SecondFactorToken: validToken, - } - encodedToken1 := "2ebcb768d0090ea4368e42880c970b61865c326172a4a2343b645cf5d7f20992" encodedToken2 := "bf355921bbf3ef3672a03e410d4194077dfa5fe863c652521763b3e7f81e7b11" invalid := []input{ @@ -936,16 +917,28 @@ func TestCSRF(t *testing.T) { } clt := s.client(t) + ctx := context.Background() // valid - _, err = s.login(clt, encodedToken1, encodedToken1, loginForm) - require.NoError(t, err) + validReq := loginWebOTPParams{ + webClient: clt, + clock: s.clock, + user: user, + password: pass, + otpSecret: otpSecret, + cookieCSRF: &encodedToken1, + headerCSRF: &encodedToken1, + } + loginWebOTP(t, ctx, validReq) // invalid for i := range invalid { - _, err := s.login(clt, invalid[i].cookieToken, invalid[i].reqToken, loginForm) - require.Error(t, err) - require.True(t, trace.IsAccessDenied(err)) + req := validReq + req.cookieCSRF = &invalid[i].cookieToken + req.headerCSRF = &invalid[i].reqToken + httpResp, _, err := rawLoginWebOTP(ctx, req) + require.NoError(t, err, "Login via /webapi/sessions/new failed unexpectedly") + assert.Equal(t, http.StatusForbidden, httpResp.StatusCode, "HTTP status code mismatch") } } @@ -960,8 +953,8 @@ func TestPasswordChange(t *testing.T) { require.NoError(t, err) req := changePasswordReq{ - OldPassword: []byte("abc123"), - NewPassword: []byte("abc1234"), + OldPassword: []byte(pack.password), + NewPassword: []byte("defabc12345678"), SecondFactorToken: validToken, } @@ -994,61 +987,95 @@ func TestValidateBearerToken(t *testing.T) { func TestWebSessionsBadInput(t *testing.T) { t.Parallel() s := newWebSuite(t) - user := "bob" - pass := "abc123" - rawSecret := "def456" - otpSecret := base32.StdEncoding.EncodeToString([]byte(rawSecret)) - err := s.server.Auth().UpsertPassword(user, []byte(pass)) + authServer := s.server.Auth() + clock := s.clock + ctx := context.Background() + + authPref, err := types.NewAuthPreference(types.AuthPreferenceSpecV2{ + Type: constants.Local, + SecondFactor: constants.SecondFactorOTP, + }) + require.NoError(t, err, "NewAuthPreference failed") + require.NoError(t, + authServer.SetAuthPreference(ctx, authPref), + "SetAuthPreference failed", + ) + + const user = "bob" + const pass = "abcdef123456" + otpSecret := newOTPSharedSecret() + badSecret := newOTPSharedSecret() + + err = authServer.UpsertPassword(user, []byte(pass)) require.NoError(t, err) dev, err := services.NewTOTPDevice("otp", otpSecret, s.clock.Now()) require.NoError(t, err) - err = s.server.Auth().UpsertMFADevice(context.Background(), user, dev) - require.NoError(t, err) - - // create valid token - validToken, err := totp.GenerateCode(otpSecret, time.Now()) + err = authServer.UpsertMFADevice(context.Background(), user, dev) require.NoError(t, err) clt := s.client(t) - reqs := []CreateSessionReq{ - // empty request - {}, - // missing user + tests := []struct { + name string + user, pass, otpSecret string + }{ + { + name: "empty request", + }, { - Pass: pass, - SecondFactorToken: validToken, + name: "missing user", + pass: pass, + otpSecret: otpSecret, }, - // missing pass { - User: user, - SecondFactorToken: validToken, + name: "missing pass", + user: user, + otpSecret: otpSecret, }, - // bad pass { - User: user, - Pass: "bla bla", - SecondFactorToken: validToken, + name: "bad pass", + user: user, + pass: "bla bla", + otpSecret: otpSecret, }, - // bad otp token { - User: user, - Pass: pass, - SecondFactorToken: "bad token", + name: "bad otp token", + user: user, + pass: pass, + otpSecret: badSecret, }, - // missing otp token { - User: user, - Pass: pass, + name: "missing otp token", + user: user, + pass: pass, }, } - 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", "web"), req) - require.Error(t, err) - require.True(t, trace.IsAccessDenied(err)) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + clock.Advance(1 * time.Minute) // Avoid OTP clashes. + + httpResp, body, err := rawLoginWebOTP(ctx, loginWebOTPParams{ + webClient: clt, + clock: clock, + user: test.user, + password: test.pass, + otpSecret: test.otpSecret, + }) + require.NoError(t, err, "HTTP request errored unexpectedly") + + // Assert HTTP response code. + assert.Equal(t, http.StatusForbidden, httpResp.StatusCode, "HTTP status mismatch") + + // Assert body error message. + var resp httpErrorResponse + require.NoError(t, + json.Unmarshal(body, &resp), + "HTTP error response unmarshal", + ) + const invalidCredentialsMessage = "invalid credentials" + assert.Contains(t, resp.Error.Message, invalidCredentialsMessage, "HTTP error message mismatch") }) } } @@ -2343,30 +2370,24 @@ func TestLogin_PrivateKeyEnabledError(t *testing.T) { require.NoError(t, err) // create user - s.createUser(t, "user1", "root", "password", "") - - loginReq, err := json.Marshal(CreateSessionReq{ - User: "user1", - Pass: "password", - }) - require.NoError(t, err) + const user = "user1" + const pass = "password1234" + s.createUser(t, user, "root", pass, "") clt := s.client(t) - 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) - csrfToken := "2ebcb768d0090ea4368e42880c970b61865c326172a4a2343b645cf5d7f20992" - addCSRFCookieToReq(req, csrfToken) - req.Header.Set("Content-Type", "application/json") - req.Header.Set(csrf.HeaderName, csrfToken) + ctx := context.Background() - re, err := clt.Client.RoundTrip(func() (*http.Response, error) { - return clt.Client.HTTPClient().Do(req) + const ua = "test-ua" + _, body, err := rawLoginWebOTP(ctx, loginWebOTPParams{ + webClient: clt, + user: user, + password: pass, + userAgent: ua, }) require.NoError(t, err) + var resErr httpErrorResponse - require.NoError(t, json.Unmarshal(re.Bytes(), &resErr)) + require.NoError(t, json.Unmarshal(body, &resErr)) require.Contains(t, resErr.Error.Message, keys.PrivateKeyPolicyHardwareKeyTouch) } @@ -2382,31 +2403,21 @@ func TestLogin(t *testing.T) { require.NoError(t, err) // create user - s.createUser(t, "user1", "root", "password", "") - - loginReq, err := json.Marshal(CreateSessionReq{ - User: "user1", - Pass: "password", - }) - require.NoError(t, err) + const user = "user1" + const pass = "password1234" + s.createUser(t, user, "root", pass, "") clt := s.client(t) - ua := "test-ua" - req, err := http.NewRequest("POST", clt.Endpoint("webapi", "sessions", "web"), bytes.NewBuffer(loginReq)) - require.NoError(t, err) - req.Header.Set("User-Agent", ua) - - csrfToken := "2ebcb768d0090ea4368e42880c970b61865c326172a4a2343b645cf5d7f20992" - addCSRFCookieToReq(req, csrfToken) - req.Header.Set("Content-Type", "application/json") - req.Header.Set(csrf.HeaderName, csrfToken) + ctx := context.Background() - re, err := clt.Client.RoundTrip(func() (*http.Response, error) { - return clt.Client.HTTPClient().Do(req) + const ua = "test-ua" + sessionResp, httpResp := loginWebOTP(t, ctx, loginWebOTPParams{ + webClient: clt, + user: user, + password: pass, + userAgent: ua, }) - require.NoError(t, err) - ctx := context.Background() events, _, err := s.server.AuthServer.AuditLog.SearchEvents(ctx, events.SearchEventsRequest{ From: s.clock.Now().Add(-time.Hour), To: s.clock.Now().Add(time.Hour), @@ -2420,11 +2431,9 @@ func TestLogin(t *testing.T) { require.Equal(t, ua, event.UserAgent) require.True(t, strings.HasPrefix(event.RemoteAddr, "127.0.0.1:")) - var rawSess *CreateSessionResponse - require.NoError(t, json.Unmarshal(re.Bytes(), &rawSess)) - cookies := re.Cookies() + cookies := httpResp.Cookies() require.Len(t, cookies, 1) - require.NotEmpty(t, rawSess.SessionExpires) + require.NotEmpty(t, sessionResp.SessionExpires) // now make sure we are logged in by calling authenticated method // we need to supply both session cookie and bearer token for @@ -2432,10 +2441,10 @@ func TestLogin(t *testing.T) { jar, err := cookiejar.New(nil) require.NoError(t, err) - clt = s.client(t, roundtrip.BearerAuth(rawSess.Token), roundtrip.CookieJar(jar)) - jar.SetCookies(s.url(), re.Cookies()) + clt = s.client(t, roundtrip.BearerAuth(sessionResp.Token), roundtrip.CookieJar(jar)) + jar.SetCookies(s.url(), cookies) - re, err = clt.Get(s.ctx, clt.Endpoint("webapi", "sites"), url.Values{}) + re, err := clt.Get(s.ctx, clt.Endpoint("webapi", "sites"), url.Values{}) require.NoError(t, err) var clusters []ui.Cluster @@ -2444,7 +2453,7 @@ func TestLogin(t *testing.T) { // in absence of session cookie or bearer auth the same request fill fail // no session cookie: - clt = s.client(t, roundtrip.BearerAuth(rawSess.Token)) + clt = s.client(t, roundtrip.BearerAuth(sessionResp.Token)) _, err = clt.Get(s.ctx, clt.Endpoint("webapi", "sites"), url.Values{}) require.Error(t, err) require.True(t, trace.IsAccessDenied(err)) @@ -4815,7 +4824,7 @@ func TestDeleteMFA(t *testing.T) { devName := devName t.Run(devName, func(t *testing.T) { t.Parallel() - otpSecret := base32.StdEncoding.EncodeToString([]byte(devName)) + otpSecret := newOTPSharedSecret() dev, err := services.NewTOTPDevice(devName, otpSecret, env.clock.Now()) require.NoError(t, err) err = env.server.Auth().UpsertMFADevice(ctx, pack.user, dev) @@ -7441,23 +7450,6 @@ func (c *TestWebClient) RoundTrip(fn roundtrip.RoundTripFn) (*roundtrip.Response return resp, err } -func (s *WebSuite) login(clt *TestWebClient, cookieToken string, reqToken string, reqData interface{}) (*roundtrip.Response, error) { - return httplib.ConvertResponse(clt.RoundTrip(func() (*http.Response, error) { - data, err := json.Marshal(reqData) - if err != nil { - return nil, err - } - req, err := http.NewRequest("POST", clt.Endpoint("webapi", "sessions", "web"), bytes.NewBuffer(data)) - if err != nil { - return nil, err - } - addCSRFCookieToReq(req, cookieToken) - req.Header.Set("Content-Type", "application/json") - req.Header.Set(csrf.HeaderName, reqToken) - return clt.HTTPClient().Do(req) - })) -} - func (s *WebSuite) loginMFA(clt *TestWebClient, reqData *client.MFAChallengeRequest, device *mocku2f.Key) (*roundtrip.Response, error) { resp, err := httplib.ConvertResponse(clt.RoundTrip(func() (*http.Response, error) { data, err := json.Marshal(reqData) @@ -7905,7 +7897,7 @@ func (r *testProxy) authPack(t *testing.T, teleportUser string, roles []types.Ro require.NoError(t, err) loginUser := u.Username - otpSecret := base32.StdEncoding.EncodeToString([]byte(rawSecret)) + otpSecret := newOTPSharedSecret() ap, err := types.NewAuthPreference(types.AuthPreferenceSpecV2{ Type: constants.Local, @@ -7918,31 +7910,22 @@ func (r *testProxy) authPack(t *testing.T, teleportUser string, roles []types.Ro r.createUser(context.Background(), t, teleportUser, loginUser, pass, otpSecret, roles) - // create a valid otp token - validToken, err := totp.GenerateCode(otpSecret, r.clock.Now()) - require.NoError(t, err) - - clt := r.newClient(t) - req := CreateSessionReq{ - User: teleportUser, - Pass: pass, - SecondFactorToken: validToken, - } - - csrfToken := "2ebcb768d0090ea4368e42880c970b61865c326172a4a2343b645cf5d7f20992" - resp := login(t, clt, csrfToken, csrfToken, req) - - var rawSession *CreateSessionResponse - require.NoError(t, json.Unmarshal(resp.Bytes(), &rawSession)) + sessionResp, httpResp := loginWebOTP(t, ctx, loginWebOTPParams{ + webClient: r.newClient(t), + clock: r.clock, + user: teleportUser, + password: pass, + otpSecret: otpSecret, + }) - session, err := rawSession.response() + session, err := sessionResp.response() require.NoError(t, err) jar, err := cookiejar.New(nil) require.NoError(t, err) - clt = r.newClient(t, roundtrip.BearerAuth(session.Token), roundtrip.CookieJar(jar)) - jar.SetCookies(&r.webURL, resp.Cookies()) + clt := r.newClient(t, roundtrip.BearerAuth(session.Token), roundtrip.CookieJar(jar)) + jar.SetCookies(&r.webURL, httpResp.Cookies()) return &authPack{ otpSecret: otpSecret, @@ -7950,7 +7933,7 @@ func (r *testProxy) authPack(t *testing.T, teleportUser string, roles []types.Ro login: loginUser, session: session, clt: clt, - cookies: resp.Cookies(), + cookies: httpResp.Cookies(), password: pass, } } @@ -8130,25 +8113,6 @@ func (r *testProxy) makeDesktopSession(t *testing.T, pack *authPack, sessionID s return ws } -func login(t *testing.T, clt *TestWebClient, cookieToken, reqToken string, reqData interface{}) *roundtrip.Response { - resp, err := httplib.ConvertResponse(clt.RoundTrip(func() (*http.Response, error) { - data, err := json.Marshal(reqData) - if err != nil { - return nil, err - } - req, err := http.NewRequest("POST", clt.Endpoint("webapi", "sessions", "web"), bytes.NewBuffer(data)) - if err != nil { - return nil, err - } - addCSRFCookieToReq(req, cookieToken) - req.Header.Set("Content-Type", "application/json") - req.Header.Set(csrf.HeaderName, reqToken) - return clt.HTTPClient().Do(req) - })) - require.NoError(t, err) - return resp -} - func validateTerminalStream(t *testing.T, ws *websocket.Conn) { t.Helper() stream := NewTerminalStream(context.Background(), ws, utils.NewLoggerForTests()) diff --git a/lib/web/login_helper_test.go b/lib/web/login_helper_test.go new file mode 100644 index 0000000000000..0e93faa64a73b --- /dev/null +++ b/lib/web/login_helper_test.go @@ -0,0 +1,158 @@ +// Teleport +// Copyright (C) 2024 Gravitational, Inc. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package web + +import ( + "bytes" + "context" + "encoding/base32" + "encoding/json" + "io" + "net/http" + "testing" + + "github.com/google/uuid" + "github.com/gravitational/trace" + "github.com/jonboulle/clockwork" + "github.com/pquerna/otp/totp" + "github.com/stretchr/testify/require" + + "github.com/gravitational/teleport/lib/httplib/csrf" +) + +// newOTPSharedSecret returns an OTP shared secret, encoded as a base32 string. +func newOTPSharedSecret() string { + secret := uuid.NewString() + return base32.StdEncoding.EncodeToString([]byte(secret)) +} + +type loginWebOTPParams struct { + webClient *TestWebClient + clock clockwork.Clock + user, password string + + // otpSecret is the shared, base32-encoded OTP secret. + // A new code is generated if provided. + // Requires clock to the set. + // If empty then no OTP is sent in the request. + otpSecret string + + userAgent string // Optional. + + cookieCSRF, headerCSRF *string // Explicit CSRF tokens. Optional. +} + +// DrainedHTTPResponse mimics an http.Response, but without a body. +type DrainedHTTPResponse struct { + StatusCode int + cookies []*http.Cookie +} + +// Cookies mimics http.Response.Cookies. +func (r *DrainedHTTPResponse) Cookies() []*http.Cookie { + return r.cookies +} + +// loginWebOTP logins the user using the /webapi/sessions/new endpoint. +// +// This is a lower-level utility for tests that want access to the returned +// unmarshaled CreateSessionResponse or HTTP response. +func loginWebOTP(t *testing.T, ctx context.Context, params loginWebOTPParams) (*CreateSessionResponse, *DrainedHTTPResponse) { + httpResp, body, err := rawLoginWebOTP(ctx, params) + require.NoError(t, err, "Login via OTP failed") + require.Equal(t, http.StatusOK, httpResp.StatusCode, "Login via OTP failed (status mismatch)") + + sessionResp := &CreateSessionResponse{} + require.NoError(t, + json.Unmarshal(body, sessionResp), + "Unmarshal failed") + return sessionResp, httpResp +} + +// rawLoginWebOTP is the raw variant of [loginWebOTP]. +// +// This is a lower-level utility for tests that want access to the response body +// itself. Callers MUST check the response status themselves, a successful login +// is not guaranteed. +// +// Note that the response body is automatically drained into a []byte and +// closed. +func rawLoginWebOTP(ctx context.Context, params loginWebOTPParams) (resp *DrainedHTTPResponse, body []byte, err error) { + webClient := params.webClient + clock := params.clock + + var code string + if params.otpSecret != "" { + code, err = totp.GenerateCode(params.otpSecret, clock.Now()) + if err != nil { + return nil, nil, trace.Wrap(err, "otp code generation") + } + } + + // Prepare request JSON body. + reqBody, err := json.Marshal(&CreateSessionReq{ + User: params.user, + Pass: params.password, + SecondFactorToken: code, + }) + if err != nil { + return nil, nil, trace.Wrap(err, "request marshal") + } + + // Prepare HTTP request. + url := webClient.Endpoint("webapi", "sessions", "web") + req, err := http.NewRequestWithContext(ctx, "POST", url, bytes.NewBuffer(reqBody)) + if err != nil { + return nil, nil, trace.Wrap(err, "create HTTP request") + } + + // Set assorted headers. + req.Header.Set("Content-Type", "application/json") + if params.userAgent != "" { + req.Header.Set("User-Agent", params.userAgent) + } + + // Set CSRF cookie and header. + const defaultCSRFToken = "2ebcb768d0090ea4368e42880c970b61865c326172a4a2343b645cf5d7f20992" + cookieCSRF := defaultCSRFToken + if params.cookieCSRF != nil { + cookieCSRF = *params.cookieCSRF + } + addCSRFCookieToReq(req, cookieCSRF) + headerCSRF := defaultCSRFToken + if params.headerCSRF != nil { + headerCSRF = *params.headerCSRF + } + req.Header.Set(csrf.HeaderName, headerCSRF) + + httpResp, err := webClient.HTTPClient().Do(req) + if err != nil { + return nil, nil, trace.Wrap(err, "do HTTP request") + } + + // Drain body, then close, then handle error. + body, err = io.ReadAll(httpResp.Body) + _ = httpResp.Body.Close() + if err != nil { + return nil, nil, trace.Wrap(err, "reading body from response") + } + + return &DrainedHTTPResponse{ + StatusCode: httpResp.StatusCode, + cookies: httpResp.Cookies(), + }, body, nil +}