From bb12e772a7fcad4c57bc083f9487ee1334da508c Mon Sep 17 00:00:00 2001 From: Dustin Decker Date: Mon, 25 Sep 2017 14:46:44 -0500 Subject: [PATCH] saml{sp,idp}: add httpOnly and secure flag (conditionally) to cookies (#116) --- samlidp/session.go | 3 ++- samlidp/session_test.go | 2 +- samlsp/middleware.go | 6 ++++-- samlsp/middleware_test.go | 10 +++++----- 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/samlidp/session.go b/samlidp/session.go index 764cc131..5e892a4b 100644 --- a/samlidp/session.go +++ b/samlidp/session.go @@ -67,7 +67,8 @@ func (s *Server) GetSession(w http.ResponseWriter, r *http.Request, req *saml.Id Name: "session", Value: session.ID, MaxAge: int(sessionMaxAge.Seconds()), - HttpOnly: false, + HttpOnly: true, + Secure: r.URL.Scheme == "https", Path: "/", }) return session diff --git a/samlidp/session_test.go b/samlidp/session_test.go index f5ec5f75..8bace035 100644 --- a/samlidp/session_test.go +++ b/samlidp/session_test.go @@ -27,7 +27,7 @@ func (test *ServerTest) TestSessionsCrud(c *C) { r.Header.Set("Content-type", "application/x-www-form-urlencoded") test.Server.ServeHTTP(w, r) c.Assert(w.Code, Equals, http.StatusOK) - c.Assert(w.Header().Get("Set-Cookie"), Equals, "session=AAIEBggKDA4QEhQWGBocHiAiJCYoKiwuMDI0Njg6PD4=; Path=/; Max-Age=3600") + c.Assert(w.Header().Get("Set-Cookie"), Equals, "session=AAIEBggKDA4QEhQWGBocHiAiJCYoKiwuMDI0Njg6PD4=; Path=/; Max-Age=3600; HttpOnly; Secure") c.Assert(string(w.Body.Bytes()), Equals, "{\"ID\":\"AAIEBggKDA4QEhQWGBocHiAiJCYoKiwuMDI0Njg6PD4=\",\"CreateTime\":\"2015-12-01T01:57:09Z\",\"ExpireTime\":\"2015-12-01T02:57:09Z\",\"Index\":\"40424446484a4c4e50525456585a5c5e60626466686a6c6e70727476787a7c7e\",\"NameID\":\"\",\"Groups\":null,\"UserName\":\"alice\",\"UserEmail\":\"\",\"UserCommonName\":\"\",\"UserSurname\":\"\",\"UserGivenName\":\"\"}\n") diff --git a/samlsp/middleware.go b/samlsp/middleware.go index 6776125c..74a2cfa3 100644 --- a/samlsp/middleware.go +++ b/samlsp/middleware.go @@ -150,7 +150,8 @@ func (m *Middleware) RequireAccount(handler http.Handler) http.Handler { Name: fmt.Sprintf("saml_%s", relayState), Value: signedState, MaxAge: int(saml.MaxIssueDelay.Seconds()), - HttpOnly: false, + HttpOnly: true, + Secure: r.URL.Scheme == "https", Path: m.ServiceProvider.AcsURL.Path, }) @@ -281,7 +282,8 @@ func (m *Middleware) Authorize(w http.ResponseWriter, r *http.Request, assertion Domain: m.CookieDomain, Value: signedToken, MaxAge: int(m.CookieMaxAge.Seconds()), - HttpOnly: false, + HttpOnly: true, + Secure: r.URL.Scheme == "https", Path: "/", }) diff --git a/samlsp/middleware_test.go b/samlsp/middleware_test.go index 0b2a51e3..ca096375 100644 --- a/samlsp/middleware_test.go +++ b/samlsp/middleware_test.go @@ -140,7 +140,7 @@ func (test *MiddlewareTest) TestRequireAccountNoCreds(c *C) { c.Assert(resp.Header().Get("Set-Cookie"), Equals, "saml_KCosLjAyNDY4Ojw-QEJERkhKTE5QUlRWWFpcXmBiZGZoamxucHJ0dnh6="+ "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6ImlkLTAwMDIwNDA2MDgwYTBjMGUxMDEyMTQxNjE4MWExYzFlMjAyMjI0MjYiLCJ1cmkiOiIvZnJvYiJ9.7f-xjK5ZzpP_51YL4aPQSQcIBKKCRb_j6CE9pZieJG0"+ - "; Path=/saml2/acs; Max-Age=90") + "; Path=/saml2/acs; Max-Age=90; HttpOnly") redirectURL, err := url.Parse(resp.Header().Get("Location")) c.Assert(err, IsNil) @@ -166,7 +166,7 @@ func (test *MiddlewareTest) TestRequireAccountNoCredsPostBinding(c *C) { c.Assert(resp.Header().Get("Set-Cookie"), Equals, "saml_KCosLjAyNDY4Ojw-QEJERkhKTE5QUlRWWFpcXmBiZGZoamxucHJ0dnh6="+ "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6ImlkLTAwMDIwNDA2MDgwYTBjMGUxMDEyMTQxNjE4MWExYzFlMjAyMjI0MjYiLCJ1cmkiOiIvZnJvYiJ9.7f-xjK5ZzpP_51YL4aPQSQcIBKKCRb_j6CE9pZieJG0"+ - "; Path=/saml2/acs; Max-Age=90") + "; Path=/saml2/acs; Max-Age=90; HttpOnly") c.Assert(string(resp.Body.Bytes()), Equals, ""+ ""+ ""+ @@ -259,7 +259,7 @@ func (test *MiddlewareTest) TestRequireAccountBadCreds(c *C) { c.Assert(resp.Header().Get("Set-Cookie"), Equals, "saml_KCosLjAyNDY4Ojw-QEJERkhKTE5QUlRWWFpcXmBiZGZoamxucHJ0dnh6="+ "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6ImlkLTAwMDIwNDA2MDgwYTBjMGUxMDEyMTQxNjE4MWExYzFlMjAyMjI0MjYiLCJ1cmkiOiIvZnJvYiJ9.7f-xjK5ZzpP_51YL4aPQSQcIBKKCRb_j6CE9pZieJG0"+ - "; Path=/saml2/acs; Max-Age=90") + "; Path=/saml2/acs; Max-Age=90; HttpOnly") redirectURL, err := url.Parse(resp.Header().Get("Location")) c.Assert(err, IsNil) decodedRequest, err := testsaml.ParseRedirectRequest(redirectURL) @@ -290,7 +290,7 @@ func (test *MiddlewareTest) TestRequireAccountExpiredCreds(c *C) { c.Assert(resp.Header().Get("Set-Cookie"), Equals, "saml_KCosLjAyNDY4Ojw-QEJERkhKTE5QUlRWWFpcXmBiZGZoamxucHJ0dnh6="+ "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6ImlkLTAwMDIwNDA2MDgwYTBjMGUxMDEyMTQxNjE4MWExYzFlMjAyMjI0MjYiLCJ1cmkiOiIvZnJvYiJ9.7f-xjK5ZzpP_51YL4aPQSQcIBKKCRb_j6CE9pZieJG0"+ - "; Path=/saml2/acs; Max-Age=90") + "; Path=/saml2/acs; Max-Age=90; HttpOnly") redirectURL, err := url.Parse(resp.Header().Get("Location")) c.Assert(err, IsNil) @@ -412,7 +412,7 @@ func (test *MiddlewareTest) TestCanParseResponse(c *C) { c.Assert(resp.Header()["Set-Cookie"], DeepEquals, []string{ "saml_KCosLjAyNDY4Ojw-QEJERkhKTE5QUlRWWFpcXmBiZGZoamxucHJ0dnh6=; Expires=Thu, 01 Jan 1970 00:00:01 GMT", "ttt=" + expectedToken + "; " + - "Path=/; Max-Age=7200", + "Path=/; Max-Age=7200; HttpOnly", }) }