set the Secure flag on auth cookies when appropriate

fixes: kiali#6912

business/authentication/openid_auth_controller.go

@@ -372,6 +372,7 @@ func (c OpenIdAuthController) redirectToAuthServerHandler(w http.ResponseWriter,
 	nonceCookie := http.Cookie{
 		Expires:  expirationTime,
 		HttpOnly: true,
+		Secure:   conf.IsServerHttps(),
 		Name:     OpenIdNonceCookieName,
 		Path:     conf.Server.WebRoot,
 		SameSite: http.SameSiteLaxMode,
@@ -487,12 +488,15 @@ func (p *openidFlowHelper) callbackCleanup(w http.ResponseWriter) *openidFlowHel
 		return p
 	}
 
+	conf := config.Get()
+
 	// Delete the nonce cookie since we no longer need it.
 	deleteNonceCookie := http.Cookie{
 		Name:     OpenIdNonceCookieName,
 		Expires:  time.Unix(0, 0),
 		HttpOnly: true,
-		Path:     config.Get().Server.WebRoot,
+		Secure:   conf.IsServerHttps(),
+		Path:     conf.Server.WebRoot,
 		SameSite: http.SameSiteStrictMode,
 		Value:    "",
 	}

business/authentication/openid_auth_controller_test.go

@@ -275,11 +275,15 @@ QwIDAQAB
 	// nonce cookie cleanup
 	assert.Equal(t, OpenIdNonceCookieName, response.Cookies()[0].Name)
 	assert.True(t, clockTime.After(response.Cookies()[0].Expires))
+	assert.True(t, response.Cookies()[0].HttpOnly)
+	assert.False(t, response.Cookies()[0].Secure)
 
 	// Session cookie
 	assert.Equal(t, AESSessionCookieName, response.Cookies()[1].Name)
 	assert.Equal(t, expectedExpiration, response.Cookies()[1].Expires)
 	assert.Equal(t, http.StatusFound, response.StatusCode)
+	assert.True(t, response.Cookies()[1].HttpOnly)
+	assert.False(t, response.Cookies()[1].Secure)
 
 	// Redirection to boot the UI
 	assert.Equal(t, "/kiali-test/", response.Header.Get("Location"))
@@ -394,6 +398,8 @@ func TestOpenIdAuthControllerAuthenticatesCorrectlyWithAuthorizationCodeFlow(t *
 	conf.LoginToken.ExpirationSeconds = 1
 	conf.Auth.OpenId.IssuerUri = testServer.URL
 	conf.Auth.OpenId.ClientId = "kiali-client"
+	conf.Identity.CertFile = "foo.cert"      // setting conf.Identity will make it look as if the endpoint ...
+	conf.Identity.PrivateKeyFile = "foo.key" // ... is HTTPS - this causes the cookies' Secure flag to be true
 	config.Set(conf)
 
 	// Returning some namespace when a cluster API call is made should have the result of
@@ -435,11 +441,15 @@ func TestOpenIdAuthControllerAuthenticatesCorrectlyWithAuthorizationCodeFlow(t *
 	// nonce cookie cleanup
 	assert.Equal(t, OpenIdNonceCookieName, response.Cookies()[0].Name)
 	assert.True(t, clockTime.After(response.Cookies()[0].Expires))
+	assert.True(t, response.Cookies()[0].HttpOnly)
+	assert.True(t, response.Cookies()[0].Secure)
 
 	// Session cookie
 	assert.Equal(t, AESSessionCookieName, response.Cookies()[1].Name)
 	assert.Equal(t, expectedExpiration, response.Cookies()[1].Expires)
 	assert.Equal(t, http.StatusFound, response.StatusCode)
+	assert.True(t, response.Cookies()[1].HttpOnly)
+	assert.True(t, response.Cookies()[1].Secure)
 
 	// Redirection to boot the UI
 	assert.Equal(t, "/kiali-test/", response.Header.Get("Location"))

business/authentication/session_persistor.go

@@ -137,6 +137,7 @@ func (p CookieSessionPersistor) CreateSession(_ *http.Request, w http.ResponseWr
 			Value:    chunk,
 			Expires:  expiresOn,
 			HttpOnly: true,
+			Secure:   conf.IsServerHttps(),
 			Path:     conf.Server.WebRoot,
 			SameSite: http.SameSiteStrictMode,
 		}
@@ -152,6 +153,7 @@ func (p CookieSessionPersistor) CreateSession(_ *http.Request, w http.ResponseWr
 			Value:    strconv.Itoa(len(sessionDataChunks)),
 			Expires:  expiresOn,
 			HttpOnly: true,
+			Secure:   conf.IsServerHttps(),
 			Path:     conf.Server.WebRoot,
 			SameSite: http.SameSiteStrictMode,
 		}
@@ -297,6 +299,7 @@ func (p CookieSessionPersistor) ReadSession(r *http.Request, w http.ResponseWrit
 // clearing any stale cookies/session.
 func (p CookieSessionPersistor) TerminateSession(r *http.Request, w http.ResponseWriter) {
 	conf := config.Get()
+
 	var cookiesToDrop []string
 
 	numChunksCookie, chunksCookieErr := r.Cookie(config.TokenCookieName + "-chunks")
@@ -326,6 +329,7 @@ func (p CookieSessionPersistor) TerminateSession(r *http.Request, w http.Respons
 				Value:    "",
 				Expires:  time.Unix(0, 0),
 				HttpOnly: true,
+				Secure:   conf.IsServerHttps(),
 				MaxAge:   -1,
 				Path:     conf.Server.WebRoot,
 				SameSite: http.SameSiteStrictMode,

business/authentication/session_persistor_test.go

@@ -19,6 +19,38 @@ type testSessionPayload struct {
 	FirstField string `json:"firstField,omitempty"`
 }
 
+// TestSecureFlag tests that the cookie Secure flag is set to true when using HTTPS.
+func TestSecureFlag(t *testing.T) {
+	cfg := config.NewConfig()
+	cfg.Server.WebRoot = "/kiali-app"
+	cfg.LoginToken.SigningKey = "kiali67890123456"
+	cfg.Identity.CertFile = "foo.cert"      // setting conf.Identity will make it look as if the endpoint ...
+	cfg.Identity.PrivateKeyFile = "foo.key" // ... is HTTPS - this causes the cookies' Secure flag to be true
+	config.Set(cfg)
+
+	clockTime := time.Date(2021, 12, 1, 0, 0, 0, 0, time.UTC)
+	util.Clock = util.ClockMock{Time: clockTime}
+
+	payload := testSessionPayload{
+		FirstField: "Foo",
+	}
+
+	rr := httptest.NewRecorder()
+	persistor := CookieSessionPersistor{}
+	expiresTime := time.Date(2021, 12, 1, 1, 0, 0, 0, time.UTC)
+	err := persistor.CreateSession(nil, rr, "test", expiresTime, payload)
+
+	response := rr.Result()
+
+	assert.Nil(t, err)
+	assert.Len(t, response.Cookies(), 1)
+
+	cookie := response.Cookies()[0]
+	assert.True(t, cookie.HttpOnly)
+	assert.True(t, true, cfg.IsServerHttps())
+	assert.True(t, cookie.Secure)
+}
+
 // TestCreateSessionNoChunks tests that the CookieSessionPersistor correctly
 // sets one cookie if the payload of a session fits in one browser cookie
 func TestCreateSessionNoChunks(t *testing.T) {
@@ -46,6 +78,8 @@ func TestCreateSessionNoChunks(t *testing.T) {
 
 	cookie := response.Cookies()[0]
 	assert.True(t, cookie.HttpOnly)
+	assert.Equal(t, false, cfg.IsServerHttps())
+	assert.Equal(t, cfg.IsServerHttps(), cookie.Secure)
 	assert.Equal(t, AESSessionCookieName, cookie.Name)
 	assert.Equal(t, "/kiali-app", cookie.Path)
 	assert.Equal(t, http.SameSiteStrictMode, cookie.SameSite)
@@ -97,6 +131,7 @@ func TestCreateSessionWithChunks(t *testing.T) {
 
 	for _, cookie := range response.Cookies() {
 		assert.True(t, cookie.HttpOnly)
+		assert.Equal(t, cfg.IsServerHttps(), cookie.Secure)
 		assert.Equal(t, "/kiali-app", cookie.Path)
 		assert.Equal(t, http.SameSiteStrictMode, cookie.SameSite)
 		assert.Equal(t, expiresTime, cookie.Expires)

config/config.go

@@ -873,6 +873,11 @@ func (conf *Config) AllNamespacesAccessible() bool {
 	return conf.Deployment.ClusterWideAccess
 }
 
+// IsServerHttps returns true if the server endpoint should use HTTPS. If false, only plaintext HTTP is supported.
+func (conf *Config) IsServerHttps() bool {
+	return conf.Identity.CertFile != "" && conf.Identity.PrivateKeyFile != ""
+}
+
 // Get the global Config
 func Get() (conf *Config) {
 	rwMutex.RLock()

server/server.go

@@ -97,10 +97,9 @@ func (s *Server) Start() {
 	conf := config.Get()
 	log.Infof("Server endpoint will start at [%v%v]", s.httpServer.Addr, conf.Server.WebRoot)
 	log.Infof("Server endpoint will serve static content from [%v]", conf.Server.StaticContentRootDirectory)
-	secure := conf.Identity.CertFile != "" && conf.Identity.PrivateKeyFile != ""
 	go func() {
 		var err error
-		if secure {
+		if conf.IsServerHttps() {
 			log.Infof("Server endpoint will require https")
 			log.Infof("Server will support protocols: %v", s.httpServer.TLSConfig.NextProtos)
 			s.router.Use(secureHttpsMiddleware)

server/server_test.go

@@ -18,6 +18,8 @@ import (
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
+
 	"github.com/kiali/kiali/business"
 	"github.com/kiali/kiali/config"
 	"github.com/kiali/kiali/config/security"
@@ -124,6 +126,8 @@ func TestAnonymousMode(t *testing.T) {
 	apiURLWithAuthentication := serverURL + "/api/authenticate"
 	apiURL := serverURL + "/api"
 
+	assert.False(t, conf.IsServerHttps())
+
 	config.Set(conf)
 
 	server := NewServer()
@@ -213,6 +217,8 @@ func TestSecureComm(t *testing.T) {
 	apiURL := serverURL + "/api"
 	metricsURL := fmt.Sprintf("http://%v:%v/", testHostname, testMetricsPort)
 
+	assert.True(t, conf.IsServerHttps())
+
 	config.Set(conf)
 
 	server := NewServer()