From c91b83969cc96bb54c5b60897e428580b937d8cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=BA=8E=E6=B9=9B?= Date: Wed, 5 Jan 2022 14:33:20 +0800 Subject: [PATCH 1/5] WIP: add cookie security configuration --- net/ghttp/ghttp_server_config.go | 12 ++++++++++ net/ghttp/ghttp_server_config_cookie.go | 23 ++++++++++++++++++ net/ghttp/ghttp_server_cookie.go | 24 ++++++++++++++++--- net/ghttp/ghttp_z_unit_feature_config_test.go | 9 +++++++ net/ghttp/ghttp_z_unit_feature_cookie_test.go | 12 ++++++++++ 5 files changed, 77 insertions(+), 3 deletions(-) diff --git a/net/ghttp/ghttp_server_config.go b/net/ghttp/ghttp_server_config.go index 7d3944718..85370b71a 100644 --- a/net/ghttp/ghttp_server_config.go +++ b/net/ghttp/ghttp_server_config.go @@ -145,6 +145,18 @@ type ServerConfig struct { // It also affects the default storage for session id. CookieDomain string `json:"cookieDomain"` + // CookieSameSite specifies cookie SameSite property. + // It also affects the default storage for session id. + CookieSameSite string `json:"sameSite"` + + // CookieSameSite specifies cookie Secure property. + // It also affects the default storage for session id. + CookieSecure bool `json:"cookieSecure"` + + // CookieSameSite specifies cookie HttpOnly property. + // It also affects the default storage for session id. + CookieHttpOnly bool `json:"CookieHttpOnly"` + // ====================================================================================================== // Session. // ====================================================================================================== diff --git a/net/ghttp/ghttp_server_config_cookie.go b/net/ghttp/ghttp_server_config_cookie.go index 266462028..2282abf05 100644 --- a/net/ghttp/ghttp_server_config_cookie.go +++ b/net/ghttp/ghttp_server_config_cookie.go @@ -7,6 +7,7 @@ package ghttp import ( + "net/http" "time" ) @@ -39,3 +40,25 @@ func (s *Server) GetCookiePath() string { func (s *Server) GetCookieDomain() string { return s.config.CookieDomain } + +// GetCookieSameSite return CookieSameSite of server. +func (s *Server) GetCookieSameSite() http.SameSite { + switch s.config.CookieSameSite { + case "lax": + return http.SameSiteLaxMode + case "none": + return http.SameSiteNoneMode + case "strict": + return http.SameSiteStrictMode + default: + return http.SameSiteDefaultMode + } +} + +func (s *Server) GetCookieSecure() bool { + return s.config.CookieSecure +} + +func (s *Server) GetCookieHttpOnly() bool { + return s.config.CookieHttpOnly +} diff --git a/net/ghttp/ghttp_server_cookie.go b/net/ghttp/ghttp_server_cookie.go index 8275593d9..d9b7ac440 100644 --- a/net/ghttp/ghttp_server_cookie.go +++ b/net/ghttp/ghttp_server_cookie.go @@ -7,6 +7,7 @@ package ghttp import ( + "github.com/gogf/gf/v2/util/gconv" "net/http" "time" @@ -88,17 +89,27 @@ func (c *Cookie) Set(key, value string) { c.request.Server.GetCookieDomain(), c.request.Server.GetCookiePath(), c.request.Server.GetCookieMaxAge(), + map[string]interface{}{ + "sameSite": c.request.Server.GetCookieSameSite(), + "secure": c.request.Server.GetCookieSecure(), + "httpOnly": c.request.Server.GetCookieHttpOnly(), + }, ) } // SetCookie sets cookie item with given domain, path and expiration age. // The optional parameter `httpOnly` specifies if the cookie item is only available in HTTP, // which is usually empty. -func (c *Cookie) SetCookie(key, value, domain, path string, maxAge time.Duration, httpOnly ...bool) { +func (c *Cookie) SetCookie(key, value, domain, path string, maxAge time.Duration, extra ...map[string]interface{}) { c.init() isHttpOnly := false - if len(httpOnly) > 0 { - isHttpOnly = httpOnly[0] + sameSite := http.SameSiteDefaultMode + secure := false + if len(extra) > 0 { + config := extra[0] + isHttpOnly = gconv.Bool(config["httpOnly"]) + sameSite = http.SameSite(gconv.Int(config["sameSite"])) + secure = gconv.Bool(config["secure"]) } httpCookie := &http.Cookie{ Name: key, @@ -106,6 +117,8 @@ func (c *Cookie) SetCookie(key, value, domain, path string, maxAge time.Duration Path: path, Domain: domain, HttpOnly: isHttpOnly, + SameSite: sameSite, + Secure: secure, } if maxAge != 0 { httpCookie.Expires = time.Now().Add(maxAge) @@ -136,6 +149,11 @@ func (c *Cookie) SetSessionId(id string) { c.request.Server.GetCookieDomain(), c.request.Server.GetCookiePath(), c.server.GetSessionCookieMaxAge(), + map[string]interface{}{ + "sameSite": c.request.Server.GetCookieSameSite(), + "secure": c.request.Server.GetCookieSecure(), + "httpOnly": c.request.Server.GetCookieHttpOnly(), + }, ) } diff --git a/net/ghttp/ghttp_z_unit_feature_config_test.go b/net/ghttp/ghttp_z_unit_feature_config_test.go index 593b6d21f..271225de2 100644 --- a/net/ghttp/ghttp_z_unit_feature_config_test.go +++ b/net/ghttp/ghttp_z_unit_feature_config_test.go @@ -28,6 +28,9 @@ func Test_ConfigFromMap(t *testing.T) { "indexFiles": g.Slice{"index.php", "main.php"}, "errorLogEnabled": true, "cookieMaxAge": "1y", + "sameSite": "lax", + "cookieSecure": true, + "CookieHttpOnly": true, } config, err := ghttp.ConfigFromMap(m) t.Assert(err, nil) @@ -38,6 +41,9 @@ func Test_ConfigFromMap(t *testing.T) { t.Assert(config.CookieMaxAge, d2) t.Assert(config.IndexFiles, m["indexFiles"]) t.Assert(config.ErrorLogEnabled, m["errorLogEnabled"]) + t.Assert(config.CookieSameSite, m["sameSite"]) + t.Assert(config.CookieSecure, m["cookieSecure"]) + t.Assert(config.CookieHttpOnly, m["CookieHttpOnly"]) }) } @@ -54,6 +60,9 @@ func Test_SetConfigWithMap(t *testing.T) { "SessionIdName": "MySessionId", "SessionPath": "/tmp/MySessionStoragePath", "SessionMaxAge": 24 * time.Hour, + "sameSite": "lax", + "cookieSecure": true, + "CookieHttpOnly": true, } s := g.Server() err := s.SetConfigWithMap(m) diff --git a/net/ghttp/ghttp_z_unit_feature_cookie_test.go b/net/ghttp/ghttp_z_unit_feature_cookie_test.go index f6d0aa0bf..e087ca88e 100644 --- a/net/ghttp/ghttp_z_unit_feature_cookie_test.go +++ b/net/ghttp/ghttp_z_unit_feature_cookie_test.go @@ -104,3 +104,15 @@ func Test_SetHttpCookie(t *testing.T) { //t.Assert(client.GetContent(ctx, "/get?k=key2"), "200") }) } + +func Test_CookieSameSite(t *testing.T) { + // todo: 补充测试 +} + +func Test_CookieSecure(t *testing.T) { + // todo: 补充测试 +} + +func Test_CookieHttpOnly(t *testing.T) { + // todo: 补充测试 +} From 572e71d76ab644b524baf8bec3c9c0b84d64ebc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=BA=8E=E6=B9=9B?= Date: Fri, 7 Jan 2022 17:10:21 +0800 Subject: [PATCH 2/5] add CookieOptions add UnitTest --- net/ghttp/ghttp_server_config.go | 4 +- net/ghttp/ghttp_server_cookie.go | 43 +++++------ net/ghttp/ghttp_z_unit_feature_config_test.go | 12 ++-- net/ghttp/ghttp_z_unit_feature_cookie_test.go | 71 +++++++++++++++++-- 4 files changed, 94 insertions(+), 36 deletions(-) diff --git a/net/ghttp/ghttp_server_config.go b/net/ghttp/ghttp_server_config.go index 85370b71a..edb955c40 100644 --- a/net/ghttp/ghttp_server_config.go +++ b/net/ghttp/ghttp_server_config.go @@ -147,7 +147,7 @@ type ServerConfig struct { // CookieSameSite specifies cookie SameSite property. // It also affects the default storage for session id. - CookieSameSite string `json:"sameSite"` + CookieSameSite string `json:"cookieSameSite"` // CookieSameSite specifies cookie Secure property. // It also affects the default storage for session id. @@ -155,7 +155,7 @@ type ServerConfig struct { // CookieSameSite specifies cookie HttpOnly property. // It also affects the default storage for session id. - CookieHttpOnly bool `json:"CookieHttpOnly"` + CookieHttpOnly bool `json:"cookieHttpOnly"` // ====================================================================================================== // Session. diff --git a/net/ghttp/ghttp_server_cookie.go b/net/ghttp/ghttp_server_cookie.go index d9b7ac440..b37492a4c 100644 --- a/net/ghttp/ghttp_server_cookie.go +++ b/net/ghttp/ghttp_server_cookie.go @@ -7,7 +7,6 @@ package ghttp import ( - "github.com/gogf/gf/v2/util/gconv" "net/http" "time" @@ -22,6 +21,13 @@ type Cookie struct { response *Response // Belonged HTTP response. } +// CookieOptions provides security config for cookies +type CookieOptions struct { + sameSite http.SameSite // cookie SameSite property + secure bool // cookie Secure property + httpOnly bool // cookie HttpOnly property +} + // cookieItem is the item stored in Cookie. type cookieItem struct { *http.Cookie // Underlying cookie items. @@ -89,10 +95,10 @@ func (c *Cookie) Set(key, value string) { c.request.Server.GetCookieDomain(), c.request.Server.GetCookiePath(), c.request.Server.GetCookieMaxAge(), - map[string]interface{}{ - "sameSite": c.request.Server.GetCookieSameSite(), - "secure": c.request.Server.GetCookieSecure(), - "httpOnly": c.request.Server.GetCookieHttpOnly(), + CookieOptions{ + sameSite: c.request.Server.GetCookieSameSite(), + secure: c.request.Server.GetCookieSecure(), + httpOnly: c.request.Server.GetCookieHttpOnly(), }, ) } @@ -100,25 +106,20 @@ func (c *Cookie) Set(key, value string) { // SetCookie sets cookie item with given domain, path and expiration age. // The optional parameter `httpOnly` specifies if the cookie item is only available in HTTP, // which is usually empty. -func (c *Cookie) SetCookie(key, value, domain, path string, maxAge time.Duration, extra ...map[string]interface{}) { +func (c *Cookie) SetCookie(key, value, domain, path string, maxAge time.Duration, options ...CookieOptions) { c.init() - isHttpOnly := false - sameSite := http.SameSiteDefaultMode - secure := false - if len(extra) > 0 { - config := extra[0] - isHttpOnly = gconv.Bool(config["httpOnly"]) - sameSite = http.SameSite(gconv.Int(config["sameSite"])) - secure = gconv.Bool(config["secure"]) + config := CookieOptions{} + if len(options) > 0 { + config = options[0] } httpCookie := &http.Cookie{ Name: key, Value: value, Path: path, Domain: domain, - HttpOnly: isHttpOnly, - SameSite: sameSite, - Secure: secure, + HttpOnly: config.httpOnly, + SameSite: config.sameSite, + Secure: config.secure, } if maxAge != 0 { httpCookie.Expires = time.Now().Add(maxAge) @@ -149,10 +150,10 @@ func (c *Cookie) SetSessionId(id string) { c.request.Server.GetCookieDomain(), c.request.Server.GetCookiePath(), c.server.GetSessionCookieMaxAge(), - map[string]interface{}{ - "sameSite": c.request.Server.GetCookieSameSite(), - "secure": c.request.Server.GetCookieSecure(), - "httpOnly": c.request.Server.GetCookieHttpOnly(), + CookieOptions{ + sameSite: c.request.Server.GetCookieSameSite(), + secure: c.request.Server.GetCookieSecure(), + httpOnly: c.request.Server.GetCookieHttpOnly(), }, ) } diff --git a/net/ghttp/ghttp_z_unit_feature_config_test.go b/net/ghttp/ghttp_z_unit_feature_config_test.go index 271225de2..78bd844e3 100644 --- a/net/ghttp/ghttp_z_unit_feature_config_test.go +++ b/net/ghttp/ghttp_z_unit_feature_config_test.go @@ -28,9 +28,9 @@ func Test_ConfigFromMap(t *testing.T) { "indexFiles": g.Slice{"index.php", "main.php"}, "errorLogEnabled": true, "cookieMaxAge": "1y", - "sameSite": "lax", + "cookieSameSite": "lax", "cookieSecure": true, - "CookieHttpOnly": true, + "cookieHttpOnly": true, } config, err := ghttp.ConfigFromMap(m) t.Assert(err, nil) @@ -41,9 +41,9 @@ func Test_ConfigFromMap(t *testing.T) { t.Assert(config.CookieMaxAge, d2) t.Assert(config.IndexFiles, m["indexFiles"]) t.Assert(config.ErrorLogEnabled, m["errorLogEnabled"]) - t.Assert(config.CookieSameSite, m["sameSite"]) + t.Assert(config.CookieSameSite, m["cookieSameSite"]) t.Assert(config.CookieSecure, m["cookieSecure"]) - t.Assert(config.CookieHttpOnly, m["CookieHttpOnly"]) + t.Assert(config.CookieHttpOnly, m["cookieHttpOnly"]) }) } @@ -60,9 +60,9 @@ func Test_SetConfigWithMap(t *testing.T) { "SessionIdName": "MySessionId", "SessionPath": "/tmp/MySessionStoragePath", "SessionMaxAge": 24 * time.Hour, - "sameSite": "lax", + "cookieSameSite": "lax", "cookieSecure": true, - "CookieHttpOnly": true, + "cookieHttpOnly": true, } s := g.Server() err := s.SetConfigWithMap(m) diff --git a/net/ghttp/ghttp_z_unit_feature_cookie_test.go b/net/ghttp/ghttp_z_unit_feature_cookie_test.go index e087ca88e..e9d30ff9c 100644 --- a/net/ghttp/ghttp_z_unit_feature_cookie_test.go +++ b/net/ghttp/ghttp_z_unit_feature_cookie_test.go @@ -9,6 +9,7 @@ package ghttp_test import ( "fmt" "net/http" + "strings" "testing" "time" @@ -105,14 +106,70 @@ func Test_SetHttpCookie(t *testing.T) { }) } -func Test_CookieSameSite(t *testing.T) { - // todo: 补充测试 +func Test_CookieOptionsDefault(t *testing.T) { + p, _ := ports.PopRand() + s := g.Server(p) + s.BindHandler("/test", func(r *ghttp.Request) { + r.Cookie.Set(r.Get("k").String(), r.Get("v").String()) + }) + s.SetPort(p) + s.SetDumpRouterMap(false) + s.Start() + defer s.Shutdown() + + time.Sleep(100 * time.Millisecond) + gtest.C(t, func(t *gtest.T) { + client := g.Client() + client.SetBrowserMode(true) + client.SetPrefix(fmt.Sprintf("http://127.0.0.1:%d", p)) + r1, e1 := client.Get(ctx, "/test?k=key1&v=100") + if r1 != nil { + defer r1.Close() + } + + t.Assert(e1, nil) + t.Assert(r1.ReadAllString(), "") + + parts := strings.Split(r1.Header.Get("Set-Cookie"), "; ") + + t.AssertEQ(len(parts), 3) + }) } -func Test_CookieSecure(t *testing.T) { - // todo: 补充测试 -} +func Test_CookieOptions(t *testing.T) { + p, _ := ports.PopRand() + s := g.Server(p) + s.SetConfigWithMap(g.Map{ + "cookieSameSite": "lax", + "cookieSecure": true, + "cookieHttpOnly": true, + }) + s.BindHandler("/test", func(r *ghttp.Request) { + r.Cookie.Set(r.Get("k").String(), r.Get("v").String()) + }) + s.SetPort(p) + s.SetDumpRouterMap(false) + s.Start() + defer s.Shutdown() -func Test_CookieHttpOnly(t *testing.T) { - // todo: 补充测试 + time.Sleep(100 * time.Millisecond) + gtest.C(t, func(t *gtest.T) { + client := g.Client() + client.SetBrowserMode(true) + client.SetPrefix(fmt.Sprintf("http://127.0.0.1:%d", p)) + r1, e1 := client.Get(ctx, "/test?k=key1&v=100") + if r1 != nil { + defer r1.Close() + } + + t.Assert(e1, nil) + t.Assert(r1.ReadAllString(), "") + + parts := strings.Split(r1.Header.Get("Set-Cookie"), "; ") + + t.AssertEQ(len(parts), 6) + t.Assert(parts[3], "HttpOnly") + t.Assert(parts[4], "Secure") + t.Assert(parts[5], "SameSite=Lax") + }) } From d045b4d2f5fa3e9957fe402927349079d3fbc36d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=BA=8E=E6=B9=9B?= Date: Fri, 7 Jan 2022 18:52:21 +0800 Subject: [PATCH 3/5] make unit test compatible with go 1.15 --- net/ghttp/ghttp_z_unit_feature_cookie_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/ghttp/ghttp_z_unit_feature_cookie_test.go b/net/ghttp/ghttp_z_unit_feature_cookie_test.go index e9d30ff9c..26652fee7 100644 --- a/net/ghttp/ghttp_z_unit_feature_cookie_test.go +++ b/net/ghttp/ghttp_z_unit_feature_cookie_test.go @@ -132,7 +132,7 @@ func Test_CookieOptionsDefault(t *testing.T) { parts := strings.Split(r1.Header.Get("Set-Cookie"), "; ") - t.AssertEQ(len(parts), 3) + t.AssertIN(len(parts), []int{3, 4}) // For go < 1.16 cookie always output "SameSite", see: https://github.com/golang/go/commit/542693e00529fbb4248fac614ece68b127a5ec4d }) } From 072d5f9760cc3f32868c2b997f8372d23331c967 Mon Sep 17 00:00:00 2001 From: yuzhan Date: Wed, 2 Mar 2022 09:56:58 +0800 Subject: [PATCH 4/5] make options public --- net/ghttp/ghttp_server_cookie.go | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/net/ghttp/ghttp_server_cookie.go b/net/ghttp/ghttp_server_cookie.go index b37492a4c..1e519d957 100644 --- a/net/ghttp/ghttp_server_cookie.go +++ b/net/ghttp/ghttp_server_cookie.go @@ -23,9 +23,9 @@ type Cookie struct { // CookieOptions provides security config for cookies type CookieOptions struct { - sameSite http.SameSite // cookie SameSite property - secure bool // cookie Secure property - httpOnly bool // cookie HttpOnly property + SameSite http.SameSite // cookie SameSite property + Secure bool // cookie Secure property + HttpOnly bool // cookie HttpOnly property } // cookieItem is the item stored in Cookie. @@ -96,15 +96,15 @@ func (c *Cookie) Set(key, value string) { c.request.Server.GetCookiePath(), c.request.Server.GetCookieMaxAge(), CookieOptions{ - sameSite: c.request.Server.GetCookieSameSite(), - secure: c.request.Server.GetCookieSecure(), - httpOnly: c.request.Server.GetCookieHttpOnly(), + SameSite: c.request.Server.GetCookieSameSite(), + Secure: c.request.Server.GetCookieSecure(), + HttpOnly: c.request.Server.GetCookieHttpOnly(), }, ) } // SetCookie sets cookie item with given domain, path and expiration age. -// The optional parameter `httpOnly` specifies if the cookie item is only available in HTTP, +// The optional parameter `options` specifies extra security configurations, // which is usually empty. func (c *Cookie) SetCookie(key, value, domain, path string, maxAge time.Duration, options ...CookieOptions) { c.init() @@ -117,9 +117,9 @@ func (c *Cookie) SetCookie(key, value, domain, path string, maxAge time.Duration Value: value, Path: path, Domain: domain, - HttpOnly: config.httpOnly, - SameSite: config.sameSite, - Secure: config.secure, + HttpOnly: config.HttpOnly, + SameSite: config.SameSite, + Secure: config.Secure, } if maxAge != 0 { httpCookie.Expires = time.Now().Add(maxAge) @@ -151,9 +151,9 @@ func (c *Cookie) SetSessionId(id string) { c.request.Server.GetCookiePath(), c.server.GetSessionCookieMaxAge(), CookieOptions{ - sameSite: c.request.Server.GetCookieSameSite(), - secure: c.request.Server.GetCookieSecure(), - httpOnly: c.request.Server.GetCookieHttpOnly(), + SameSite: c.request.Server.GetCookieSameSite(), + Secure: c.request.Server.GetCookieSecure(), + HttpOnly: c.request.Server.GetCookieHttpOnly(), }, ) } From 3bff71b3fc412a898807a75ca9d9fa0573a18bfe Mon Sep 17 00:00:00 2001 From: yuzhan Date: Wed, 2 Mar 2022 15:33:58 +0800 Subject: [PATCH 5/5] merge master and update unit test --- net/ghttp/ghttp_z_unit_feature_cookie_test.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/net/ghttp/ghttp_z_unit_feature_cookie_test.go b/net/ghttp/ghttp_z_unit_feature_cookie_test.go index 4271165cb..b6aa8fa8c 100644 --- a/net/ghttp/ghttp_z_unit_feature_cookie_test.go +++ b/net/ghttp/ghttp_z_unit_feature_cookie_test.go @@ -104,12 +104,10 @@ func Test_SetHttpCookie(t *testing.T) { } func Test_CookieOptionsDefault(t *testing.T) { - p, _ := ports.PopRand() - s := g.Server(p) + s := g.Server(guid.S()) s.BindHandler("/test", func(r *ghttp.Request) { r.Cookie.Set(r.Get("k").String(), r.Get("v").String()) }) - s.SetPort(p) s.SetDumpRouterMap(false) s.Start() defer s.Shutdown() @@ -118,7 +116,7 @@ func Test_CookieOptionsDefault(t *testing.T) { gtest.C(t, func(t *gtest.T) { client := g.Client() client.SetBrowserMode(true) - client.SetPrefix(fmt.Sprintf("http://127.0.0.1:%d", p)) + client.SetPrefix(fmt.Sprintf("http://127.0.0.1:%d", s.GetListenedPort())) r1, e1 := client.Get(ctx, "/test?k=key1&v=100") if r1 != nil { defer r1.Close() @@ -134,8 +132,7 @@ func Test_CookieOptionsDefault(t *testing.T) { } func Test_CookieOptions(t *testing.T) { - p, _ := ports.PopRand() - s := g.Server(p) + s := g.Server(guid.S()) s.SetConfigWithMap(g.Map{ "cookieSameSite": "lax", "cookieSecure": true, @@ -144,7 +141,6 @@ func Test_CookieOptions(t *testing.T) { s.BindHandler("/test", func(r *ghttp.Request) { r.Cookie.Set(r.Get("k").String(), r.Get("v").String()) }) - s.SetPort(p) s.SetDumpRouterMap(false) s.Start() defer s.Shutdown() @@ -153,7 +149,7 @@ func Test_CookieOptions(t *testing.T) { gtest.C(t, func(t *gtest.T) { client := g.Client() client.SetBrowserMode(true) - client.SetPrefix(fmt.Sprintf("http://127.0.0.1:%d", p)) + client.SetPrefix(fmt.Sprintf("http://127.0.0.1:%d", s.GetListenedPort())) r1, e1 := client.Get(ctx, "/test?k=key1&v=100") if r1 != nil { defer r1.Close()