From 27acf0ef011cced48381d43928adc98ba17aaabf Mon Sep 17 00:00:00 2001 From: jholdstock Date: Tue, 15 Aug 2023 10:24:49 +0100 Subject: [PATCH] webapi: Add test for securecookie error. A simple test to ensure an error string returned by the securecookie lib does not change when we upgrade to newer versions. Important because vspd checks for this error using string comparison. --- webapi/middleware.go | 5 ++- webapi/middleware_test.go | 67 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 webapi/middleware_test.go diff --git a/webapi/middleware.go b/webapi/middleware.go index 0e0c6197..7e18df28 100644 --- a/webapi/middleware.go +++ b/webapi/middleware.go @@ -23,6 +23,9 @@ import ( "golang.org/x/time/rate" ) +// This is a hard-coded string from the securecookie library. +const invalidCookieErr = "securecookie: the value is not valid" + // rateLimit middleware limits how many requests each client IP can submit per // second. If the limit is exceeded the limitExceeded handler will be executed // and the context will be aborted. @@ -67,7 +70,7 @@ func (s *Server) withSession(store *sessions.CookieStore) gin.HandlerFunc { // "value is not valid" occurs if the cookie secret changes. This is // common during development (eg. when using the test harness) but // it should not occur in production. - if strings.Contains(err.Error(), "securecookie: the value is not valid") { + if strings.Contains(err.Error(), invalidCookieErr) { s.log.Warn("Cookie secret has changed. Generating new session.") // Persist the newly generated session. diff --git a/webapi/middleware_test.go b/webapi/middleware_test.go new file mode 100644 index 00000000..ec8d32fc --- /dev/null +++ b/webapi/middleware_test.go @@ -0,0 +1,67 @@ +// Copyright (c) 2023 The Decred developers +// Use of this source code is governed by an ISC +// license that can be found in the LICENSE file. + +package webapi + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/gorilla/sessions" +) + +// NOTE: This test does not test any vspd code. +// +// If the cookie store secret changes unexpectedly (common during development) +// the securecookie library returns an error with a hard-coded, non-exported +// string. +// +// "securecookie: the value is not valid" +// +// TestCookieSecretError ensures the string returned by the lib does not change, +// which is important because vspd checks for the error using string comparison. +func TestCookieSecretError(t *testing.T) { + + // Create a cookie store, get a cookie from it. + + store := sessions.NewCookieStore([]byte("first secret")) + + req, err := http.NewRequest(http.MethodPost, "/", nil) + if err != nil { + t.Fatal(err) + } + + session, err := store.Get(req, "key") + if err != nil { + t.Fatal(err) + } + + w := httptest.NewRecorder() + err = store.Save(req, w, session) + if err != nil { + t.Fatal(err) + } + + cookie := w.Result().Header["Set-Cookie"][0] + + // Create another cookie store using a different secret, send cookie from + // first store to the new store, confirm error is correct. + + store2 := sessions.NewCookieStore([]byte("second secret")) + + req2, err := http.NewRequest(http.MethodPost, "/", nil) + if err != nil { + t.Fatal(err) + } + req2.Header.Add("Cookie", cookie) + + _, err = store2.Get(req2, "key") + + if !strings.Contains(err.Error(), invalidCookieErr) { + t.Fatalf("securecookie library returned unexpected error, wanted %q, got %q", + invalidCookieErr, err.Error()) + } +}