Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor cookie package to no longer depend on Echo #3457

Merged
merged 2 commits into from
Nov 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions pkg/oauth/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (s *server) authCookie() *cookie.Cookie {
var errAuthCookie = errors.DefineUnauthenticated("auth_cookie", "could not get auth cookie")

func (s *server) getAuthCookie(c echo.Context) (cookie auth.CookieShape, err error) {
ok, err := s.authCookie().Get(c, &cookie)
ok, err := s.authCookie().Get(c.Response(), c.Request(), &cookie)
if err != nil {
return cookie, err
}
Expand All @@ -58,18 +58,18 @@ func (s *server) getAuthCookie(c echo.Context) (cookie auth.CookieShape, err err

func (s *server) updateAuthCookie(c echo.Context, update func(value *auth.CookieShape) error) error {
cookie := &auth.CookieShape{}
_, err := s.authCookie().Get(c, cookie)
_, err := s.authCookie().Get(c.Response(), c.Request(), cookie)
if err != nil {
return err
}
if err = update(cookie); err != nil {
return err
}
return s.authCookie().Set(c, cookie)
return s.authCookie().Set(c.Response(), c.Request(), cookie)
}

func (s *server) removeAuthCookie(c echo.Context) {
s.authCookie().Remove(c)
s.authCookie().Remove(c.Response(), c.Request())
}

const userSessionKey = "user_session"
Expand Down
54 changes: 14 additions & 40 deletions pkg/web/cookie/cookie.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,13 @@
package cookie

import (
"fmt"
"net/http"
"time"

"github.com/gorilla/securecookie"
echo "github.com/labstack/echo/v4"
"go.thethings.network/lorawan-stack/v3/pkg/webmiddleware"
)

const (
// encoderKey is the key where the encoder will be stored on the request.
encoderKey = "cookie.encoder"

// tombstone is the cookie tombstone value.
tombstone = "<deleted>"
)
Expand All @@ -46,51 +41,30 @@ type Cookie struct {
HTTPOnly bool
}

// Cookies is a middleware function that makes the handlers capable of handling cookies via
// methods of this package.
func Cookies(hashKey, blockKey []byte) echo.MiddlewareFunc {
s := securecookie.New(hashKey, blockKey)
return func(next echo.HandlerFunc) echo.HandlerFunc {
return func(c echo.Context) error {
c.Set(encoderKey, s)
return next(c)
}
}
}

func getConfig(c echo.Context) (*securecookie.SecureCookie, error) {
encoder, _ := c.Get(encoderKey).(*securecookie.SecureCookie)
if encoder == nil {
return nil, fmt.Errorf("No cookie.encoder set")
}

return encoder, nil
}

// Get decodes the cookie into the value. Returns false if the cookie is not there.
func (d *Cookie) Get(c echo.Context, v interface{}) (bool, error) {
s, err := getConfig(c)
func (d *Cookie) Get(w http.ResponseWriter, r *http.Request, v interface{}) (bool, error) {
s, err := webmiddleware.GetSecureCookie(r.Context())
if err != nil {
return false, err
}

cookie, err := c.Request().Cookie(d.Name)
cookie, err := r.Cookie(d.Name)
if err != nil || cookie.Value == tombstone {
return false, nil
}

err = s.Decode(d.Name, cookie.Value, v)
if err != nil {
d.Remove(c)
d.Remove(w, r)
return false, nil
}

return true, nil
}

// Set the value of the cookie.
func (d *Cookie) Set(c echo.Context, v interface{}) error {
s, err := getConfig(c)
func (d *Cookie) Set(w http.ResponseWriter, r *http.Request, v interface{}) error {
s, err := webmiddleware.GetSecureCookie(r.Context())
if err != nil {
return err
}
Expand All @@ -100,7 +74,7 @@ func (d *Cookie) Set(c echo.Context, v interface{}) error {
return err
}

http.SetCookie(c.Response().Writer, &http.Cookie{
http.SetCookie(w, &http.Cookie{
Name: d.Name,
Path: d.Path,
MaxAge: int(d.MaxAge.Nanoseconds() / 1000),
Expand All @@ -112,18 +86,18 @@ func (d *Cookie) Set(c echo.Context, v interface{}) error {
}

// Exists checks if the cookies exists.
func (d *Cookie) Exists(c echo.Context) bool {
cookie, err := c.Request().Cookie(d.Name)
func (d *Cookie) Exists(r *http.Request) bool {
cookie, err := r.Cookie(d.Name)
return err == nil && cookie.Value != tombstone
}

// Remove the cookie with the specified name (if it exists).
func (d *Cookie) Remove(c echo.Context) {
if !d.Exists(c) {
func (d *Cookie) Remove(w http.ResponseWriter, r *http.Request) {
if !d.Exists(r) {
return
}

http.SetCookie(c.Response().Writer, &http.Cookie{
http.SetCookie(w, &http.Cookie{
Name: d.Name,
Path: d.Path,
HttpOnly: d.HTTPOnly,
Expand All @@ -135,7 +109,7 @@ func (d *Cookie) Remove(c echo.Context) {
// Additionally, explicitly remove the cookie also from the current path.
// This can be necessary to also remove old cookies when the cookie paths
// have been changed.
http.SetCookie(c.Response().Writer, &http.Cookie{
http.SetCookie(w, &http.Cookie{
Name: d.Name,
HttpOnly: d.HTTPOnly,
Value: tombstone,
Expand Down
44 changes: 20 additions & 24 deletions pkg/web/cookie/cookie_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,11 @@ import (
"net/http/httptest"
"testing"

echo "github.com/labstack/echo/v4"
"github.com/gorilla/mux"
"github.com/smartystreets/assertions"
"go.thethings.network/lorawan-stack/v3/pkg/random"
"go.thethings.network/lorawan-stack/v3/pkg/util/test/assertions/should"
"go.thethings.network/lorawan-stack/v3/pkg/webmiddleware"
"golang.org/x/net/publicsuffix"
)

Expand All @@ -36,54 +37,49 @@ type testCookieValue struct {
Value string
}

func testSetCookie(t *testing.T, value string) echo.HandlerFunc {
func testSetCookie(t *testing.T, value string) http.HandlerFunc {
a := assertions.New(t)
return func(c echo.Context) error {
err := testCookieSettings.Set(c, &testCookieValue{
return func(w http.ResponseWriter, r *http.Request) {
err := testCookieSettings.Set(w, r, &testCookieValue{
Value: value,
})
a.So(err, should.BeNil)

return err
}
}

func testGetCookie(t *testing.T, value string, exists bool) echo.HandlerFunc {
func testGetCookie(t *testing.T, value string, exists bool) http.HandlerFunc {
a := assertions.New(t)
return func(c echo.Context) error {
return func(w http.ResponseWriter, r *http.Request) {
var v testCookieValue
ok, err := testCookieSettings.Get(c, &v)
ok, err := testCookieSettings.Get(w, r, &v)
a.So(err, should.BeNil)
a.So(ok, should.Equal, exists)
a.So(v.Value, should.Equal, value)

present := testCookieSettings.Exists(c)
present := testCookieSettings.Exists(r)
a.So(present, should.Equal, exists)

return err
}
}

func testDeleteCookie() echo.HandlerFunc {
return func(c echo.Context) error {
testCookieSettings.Remove(c)

return nil
func testDeleteCookie() http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
testCookieSettings.Remove(w, r)
}
}

func TestCookie(t *testing.T) {
e := echo.New()
root := mux.NewRouter()

a := assertions.New(t)
blockKey := random.Bytes(32)
hashKey := random.Bytes(64)

e.Use(Cookies(hashKey, blockKey))
root.Use(mux.MiddlewareFunc(webmiddleware.Cookies(hashKey, blockKey)))

e.GET("/set", testSetCookie(t, "test_value"))
e.GET("/get", testGetCookie(t, "test_value", true))
e.GET("/del", testDeleteCookie())
e.GET("/no_cookie", testGetCookie(t, "", false))
root.Path("/set").HandlerFunc(testSetCookie(t, "test_value")).Methods(http.MethodGet)
root.Path("/get").HandlerFunc(testGetCookie(t, "test_value", true)).Methods(http.MethodGet)
root.Path("/del").HandlerFunc(testDeleteCookie()).Methods(http.MethodGet)
root.Path("/no_cookie").HandlerFunc(testGetCookie(t, "", false)).Methods(http.MethodGet)

jar, err := cookiejar.New(&cookiejar.Options{PublicSuffixList: publicsuffix.List})
if err != nil {
Expand All @@ -99,7 +95,7 @@ func TestCookie(t *testing.T) {

rec := httptest.NewRecorder()

e.ServeHTTP(rec, req)
root.ServeHTTP(rec, req)

resp := rec.Result()
resp.Request = req
Expand Down
6 changes: 3 additions & 3 deletions pkg/web/oauthclient/cookie_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ type authCookie struct {

func (oc *OAuthClient) getAuthCookie(c echo.Context) (authCookie, error) {
value := authCookie{}
ok, err := oc.AuthCookie().Get(c, &value)
ok, err := oc.AuthCookie().Get(c.Response(), c.Request(), &value)
if err != nil {
return authCookie{}, err
}
Expand All @@ -56,9 +56,9 @@ func (oc *OAuthClient) getAuthCookie(c echo.Context) (authCookie, error) {
}

func (oc *OAuthClient) setAuthCookie(c echo.Context, value authCookie) error {
return oc.AuthCookie().Set(c, value)
return oc.AuthCookie().Set(c.Response(), c.Request(), value)
}

func (oc *OAuthClient) removeAuthCookie(c echo.Context) {
oc.AuthCookie().Remove(c)
oc.AuthCookie().Remove(c.Response(), c.Request())
}
6 changes: 3 additions & 3 deletions pkg/web/oauthclient/cookie_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func newState(next string) state {

func (oc *OAuthClient) getStateCookie(c echo.Context) (state, error) {
s := state{}
ok, err := oc.StateCookie().Get(c, &s)
ok, err := oc.StateCookie().Get(c.Response(), c.Request(), &s)
if err != nil {
return s, echo.NewHTTPError(http.StatusBadRequest, "Invalid state cookie")
}
Expand All @@ -66,9 +66,9 @@ func (oc *OAuthClient) getStateCookie(c echo.Context) (state, error) {
}

func (oc *OAuthClient) setStateCookie(c echo.Context, value state) error {
return oc.StateCookie().Set(c, value)
return oc.StateCookie().Set(c.Response(), c.Request(), value)
}

func (oc *OAuthClient) removeStateCookie(c echo.Context) {
oc.StateCookie().Remove(c)
oc.StateCookie().Remove(c.Response(), c.Request())
}
2 changes: 0 additions & 2 deletions pkg/web/web.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import (
"go.thethings.network/lorawan-stack/v3/pkg/fillcontext"
"go.thethings.network/lorawan-stack/v3/pkg/log"
"go.thethings.network/lorawan-stack/v3/pkg/random"
"go.thethings.network/lorawan-stack/v3/pkg/web/cookie"
"go.thethings.network/lorawan-stack/v3/pkg/webhandlers"
"go.thethings.network/lorawan-stack/v3/pkg/webmiddleware"
"go.thethings.network/lorawan-stack/v3/pkg/webui"
Expand Down Expand Up @@ -252,7 +251,6 @@ func New(ctx context.Context, opts ...Option) (*Server, error) {

server.Use(
echomiddleware.Gzip(),
cookie.Cookies(hashKey, blockKey),
)

s := &Server{
Expand Down
4 changes: 2 additions & 2 deletions pkg/webmiddleware/cookie.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ func Cookies(hashKey, blockKey []byte) MiddlewareFunc {
var errInvalidContext = errors.DefineInternal("secure_cookie_invalid_context", "No secure cookie value in this context")

// GetSecureCookie retrieves the secure cookie encoder from the context.
func GetSecureCookie(ctx context.Context) (secureCookie *securecookie.SecureCookie, err error) {
secureCookie = ctx.Value(secureCookieCtxKey).(*securecookie.SecureCookie)
func GetSecureCookie(ctx context.Context) (*securecookie.SecureCookie, error) {
secureCookie, _ := ctx.Value(secureCookieCtxKey).(*securecookie.SecureCookie)
if secureCookie == nil {
return nil, errInvalidContext
}
Expand Down