saml{sp,idp}: add httpOnly and secure flag (conditionally) to cookies (…

…#116)
crewjam · Sep 25, 2017 · bb12e77 · bb12e77
1 parent 50777a1
commit bb12e77
Show file tree

Hide file tree

Showing 4 changed files with 12 additions and 9 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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, ""+
 		"<!DOCTYPE html>"+
 		"<html>"+
@@ -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",
 	})
 }