github/zot
1
0
Fork 0
mirror of https://github.com/project-zot/zot.git synced 2026-06-17 21:17:58 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix: configure cookie Secure flag based on TLS configuration (#3482)

Browse Source 
Make the Secure flag for session cookies configurable based on Zot's
TLS settings. This allows cookies to work properly when Zot is
accessed over HTTP (without TLS).

Changes:
- Add SecureSession field to AuthConfig to allow explicit control
- Add UseSecureSession() method that returns true when TLS is
  configured, or uses SecureSession setting if provided
- Update saveUserLoggedSession() to accept and use secure parameter
- Add tests for UseSecureSession() in config_test.go
- Enhance authn tests to verify cookie Secure flag behavior
- Fix TestAuthnSessionErrors by creating new client without cookies

The logic is:
- If TLS is configured, cookies always have Secure=true
- If TLS is not configured but SecureSession is explicitly set,
  use that value
- Otherwise, default to Secure=false for HTTP-only deployments

Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>
This commit is contained in:
Andrei Aaron
2025-10-27 17:21:21 +02:00
committed by GitHub
parent 22cfd9430b
commit 029f6f0a29
5 changed files with 318 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/api/authn.go
+9 -6
View File
@@ -135,7 +135,8 @@ func (amw *AuthnMiddleware) basicAuthn(ctlr *Controller, userAc *reqCtx.UserAcce
// saved logged session only if the request comes from web (has UI session header value)
if hasSessionHeader(request) {
if err := saveUserLoggedSession(cookieStore, response, request, identity, ctlr.Log); err != nil {
secure := ctlr.Config.UseSecureSession()
if err := saveUserLoggedSession(cookieStore, response, request, identity, secure, ctlr.Log); err != nil {
return false, err
}
}
@@ -173,7 +174,8 @@ func (amw *AuthnMiddleware) basicAuthn(ctlr *Controller, userAc *reqCtx.UserAcce
// saved logged session only if the request comes from web (has UI session header value)
if hasSessionHeader(request) {
if err := saveUserLoggedSession(cookieStore, response, request, identity, ctlr.Log); err != nil {
secure := ctlr.Config.UseSecureSession()
if err := saveUserLoggedSession(cookieStore, response, request, identity, secure, ctlr.Log); err != nil {
return false, err
}
}
@@ -787,11 +789,11 @@ func GetGithubUserInfo(ctx context.Context, client *github.Client, log log.Logge
}
func saveUserLoggedSession(cookieStore sessions.Store, response http.ResponseWriter,
request *http.Request, identity string, log log.Logger,
request *http.Request, identity string, secure bool, log log.Logger,
) error {
session, _ := cookieStore.Get(request, "session")
session.Options.Secure = true
session.Options.Secure = secure
session.Options.HttpOnly = true
session.Options.SameSite = http.SameSiteDefaultMode
session.Values["authStatus"] = true
@@ -806,7 +808,7 @@ func saveUserLoggedSession(cookieStore sessions.Store, response http.ResponseWri
}
userInfoCookie := sessions.NewCookie("user", identity, &sessions.Options{
Secure: true,
Secure: secure,
HttpOnly: false,
MaxAge: cookiesMaxAge,
SameSite: http.SameSiteDefaultMode,
@@ -846,7 +848,8 @@ func OAuth2Callback(ctlr *Controller, w http.ResponseWriter, r *http.Request, st
// if this line has been reached, then a new session should be created
// if the `session` key is already on the cookie, it's not a valid one
if err := saveUserLoggedSession(ctlr.CookieStore, w, r, email, ctlr.Log); err != nil {
secure := ctlr.Config.UseSecureSession()
if err := saveUserLoggedSession(ctlr.CookieStore, w, r, email, secure, ctlr.Log); err != nil {
return "", err
}
pkg/api/authn_test.go
+200 -7
View File
@@ -5,6 +5,8 @@ package api_test
import (
"context"
"crypto/tls"
"crypto/x509"
"encoding/json"
"errors"
"io/fs"
@@ -42,6 +44,11 @@ import (
var ErrUnexpectedError = errors.New("error: unexpected error")
const (
sessionCookieName = "session"
userCookieName = "user"
)
type (
apiKeyResponse struct {
mTypes.APIKeyDetails
@@ -237,18 +244,21 @@ func TestAPIKeys(t *testing.T) {
client := resty.New()
client.SetRedirectPolicy(test.CustomRedirectPolicy(20))
// first login user
// first login user - in OAuth2 flow, redirect policy automatically follows and gets cookies
resp, err := client.R().
SetHeader(constants.SessionClientHeaderName, constants.SessionClientHeaderValue).
SetQueryParam("provider", "oidc").
Get(baseURL + constants.LoginPath)
So(err, ShouldBeNil)
So(resp, ShouldNotBeNil)
So(resp.StatusCode(), ShouldEqual, http.StatusCreated)
cookies := resp.Cookies()
// call endpoint without session
resp, err = client.R().
// call endpoint without session - use a new client without cookies
clientWithoutSession := resty.New()
resp, err = clientWithoutSession.R().
SetBody(reqBody).
SetHeader(constants.SessionClientHeaderName, constants.SessionClientHeaderValue).
Post(baseURL + constants.APIKeyPath)
So(err, ShouldBeNil)
@@ -388,7 +398,10 @@ func TestAPIKeys(t *testing.T) {
So(resp, ShouldNotBeNil)
So(resp.StatusCode(), ShouldEqual, http.StatusCreated)
client.SetCookies(resp.Cookies())
cookies := resp.Cookies()
verifySessionCookiesSecureFlag(cookies, false) // No TLS configured
client.SetCookies(cookies)
// call endpoint with session ( added to client after previous request)
resp, err = client.R().
@@ -518,7 +531,10 @@ func TestAPIKeys(t *testing.T) {
So(resp, ShouldNotBeNil)
So(resp.StatusCode(), ShouldEqual, http.StatusCreated)
client.SetCookies(resp.Cookies())
cookies = resp.Cookies()
verifySessionCookiesSecureFlag(cookies, false) // No TLS configured
client.SetCookies(cookies)
resp, err = client.R().
SetBody(reqBody).
@@ -782,7 +798,10 @@ func TestAPIKeys(t *testing.T) {
So(resp, ShouldNotBeNil)
So(resp.StatusCode(), ShouldEqual, http.StatusCreated)
client.SetCookies(resp.Cookies())
cookies := resp.Cookies()
verifySessionCookiesSecureFlag(cookies, false) // No TLS configured
client.SetCookies(cookies)
// call endpoint with session ( added to client after previous request)
resp, err = client.R().
@@ -817,7 +836,10 @@ func TestAPIKeys(t *testing.T) {
So(resp, ShouldNotBeNil)
So(resp.StatusCode(), ShouldEqual, http.StatusCreated)
client.SetCookies(resp.Cookies())
cookies := resp.Cookies()
verifySessionCookiesSecureFlag(cookies, false) // No TLS configured
client.SetCookies(cookies)
// call endpoint with session ( added to client after previous request)
resp, err = client.R().
@@ -1112,6 +1134,159 @@ func TestCookiestoreCleanup(t *testing.T) {
})
}
func TestCookieSecureFlag(t *testing.T) {
Convey("Test cookie Secure flag based on configuration", t, func() {
const (
serverCertPath = "../../test/data/server.cert"
serverKeyPath = "../../test/data/server.key"
caCertPath = "../../test/data/ca.crt"
)
mockOIDCServer, err := authutils.MockOIDCRun()
So(err, ShouldBeNil)
defer func() {
err := mockOIDCServer.Shutdown()
So(err, ShouldBeNil)
}()
username, _ := test.GenerateRandomString()
password, _ := test.GenerateRandomString()
htpasswdPath := test.MakeHtpasswdFileFromString(test.GetCredString(username, password))
defer os.Remove(htpasswdPath)
mockOIDCConfig := mockOIDCServer.Config()
defaultVal := true
Convey("Test with TLS configured - cookies should be Secure=true", func() {
conf := config.New()
port := test.GetFreePort()
baseURL := test.GetSecureBaseURL(port)
conf.HTTP.Port = port
conf.HTTP.TLS = &config.TLSConfig{
Cert: serverCertPath,
Key: serverKeyPath,
}
conf.HTTP.Auth = &config.AuthConfig{
HTPasswd: config.AuthHTPasswd{
Path: htpasswdPath,
},
OpenID: &config.OpenIDConfig{
Providers: map[string]config.OpenIDProviderConfig{
"oidc": {
ClientID: mockOIDCConfig.ClientID,
ClientSecret: mockOIDCConfig.ClientSecret,
KeyPath: "",
Issuer: mockOIDCConfig.Issuer,
Scopes: []string{"openid", "email", "groups"},
},
},
},
APIKey: defaultVal,
}
ctlr := api.NewController(conf)
ctlr.Config.Storage.RootDirectory = t.TempDir()
cm := test.NewControllerManager(ctlr)
cm.StartServer()
defer cm.StopServer()
// Load CA certificate for proper TLS verification
caCert, err := os.ReadFile(caCertPath)
So(err, ShouldBeNil)
caCertPool := x509.NewCertPool()
caCertPool.AppendCertsFromPEM(caCert)
client := resty.New()
client.SetRedirectPolicy(test.CustomRedirectPolicy(20))
client.SetTLSClientConfig(&tls.Config{RootCAs: caCertPool, MinVersion: tls.VersionTLS12})
for {
if _, err := client.R().Get(baseURL); err == nil {
break
}
// wait for server to be ready
time.Sleep(test.SleepTime)
}
resp, err := client.R().
SetHeader(constants.SessionClientHeaderName, constants.SessionClientHeaderValue).
SetQueryParam("provider", "oidc").
Get(baseURL + constants.LoginPath)
So(err, ShouldBeNil)
So(resp, ShouldNotBeNil)
cookies := resp.Cookies()
So(len(cookies), ShouldBeGreaterThan, 0)
// Verify cookies have Secure=true when TLS is configured
verifySessionCookiesSecureFlag(cookies, true)
})
Convey("Test with SecureSession=true configured - cookies should be Secure=true", func() {
secureTrue := true
conf := config.New()
port := test.GetFreePort()
baseURL := test.GetBaseURL(port)
conf.HTTP.Port = port
conf.HTTP.TLS = nil // No TLS
conf.HTTP.Auth = &config.AuthConfig{
HTPasswd: config.AuthHTPasswd{
Path: htpasswdPath,
},
OpenID: &config.OpenIDConfig{
Providers: map[string]config.OpenIDProviderConfig{
"oidc": {
ClientID: mockOIDCConfig.ClientID,
ClientSecret: mockOIDCConfig.ClientSecret,
KeyPath: "",
Issuer: mockOIDCConfig.Issuer,
Scopes: []string{"openid", "email", "groups"},
},
},
},
APIKey: defaultVal,
SecureSession: &secureTrue,
}
ctlr := api.NewController(conf)
ctlr.Config.Storage.RootDirectory = t.TempDir()
cm := test.NewControllerManager(ctlr)
cm.StartServer()
defer cm.StopServer()
test.WaitTillServerReady(baseURL)
client := resty.New()
client.SetRedirectPolicy(test.CustomRedirectPolicy(20))
// login user
resp, err := client.R().
SetHeader(constants.SessionClientHeaderName, constants.SessionClientHeaderValue).
SetQueryParam("provider", "oidc").
Get(baseURL + constants.LoginPath)
So(err, ShouldBeNil)
So(resp, ShouldNotBeNil)
cookies := resp.Cookies()
So(len(cookies), ShouldBeGreaterThan, 0)
// Verify cookies have Secure=true when SecureSession is set to true
verifySessionCookiesSecureFlag(cookies, true)
})
})
}
func TestRedisCookieStore(t *testing.T) {
log := log.Logger{}
@@ -1268,3 +1443,21 @@ func (di badDirInfo) Name() string {
func (di badDirInfo) String() string {
return fs.FormatDirEntry(di)
}
func verifySessionCookiesSecureFlag(cookies []*http.Cookie, expectedSecure bool) {
var sessionCookie, userCookie *http.Cookie
for _, cookie := range cookies {
if cookie.Name == sessionCookieName {
sessionCookie = cookie
} else if cookie.Name == userCookieName {
userCookie = cookie
}
}
// Verify both cookies exist and have correct Secure flag
So(sessionCookie, ShouldNotBeNil)
So(userCookie, ShouldNotBeNil)
So(sessionCookie.Secure, ShouldEqual, expectedSecure)
So(userCookie.Secure, ShouldEqual, expectedSecure)
}
pkg/api/config/config.go
+27
View File
@@ -80,6 +80,7 @@ type AuthConfig struct {
SessionHashKey []byte `json:"-"`
SessionEncryptKey []byte `json:"-"`
SessionDriver map[string]any `mapstructure:",omitempty"`
SecureSession *bool `json:"secureSession,omitempty" mapstructure:"secureSession,omitempty"`
}
// IsLdapAuthEnabled checks if LDAP authentication is enabled in this auth config.
@@ -670,6 +671,7 @@ func (c *Config) UpdateReloadableConfig(newConfig *Config) {
c.HTTP.Auth.LDAP = newConfig.HTTP.Auth.LDAP
c.HTTP.Auth.APIKey = newConfig.HTTP.Auth.APIKey
c.HTTP.Auth.OpenID = newConfig.HTTP.Auth.OpenID
c.HTTP.Auth.SecureSession = newConfig.HTTP.Auth.SecureSession
}
// Initialize and update AccessControlConfig
@@ -1016,6 +1018,31 @@ func (c *Config) IsCompatEnabled() bool {
return len(c.HTTP.Compat) > 0
}
// UseSecureSession returns whether cookies should have the Secure flag set.
// If TLS is configured, always returns true. Otherwise, returns the value
// of SecureSession if set, or false by default.
func (c *Config) UseSecureSession() bool {
if c == nil {
return false
}
c.mu.RLock()
defer c.mu.RUnlock()
// If TLS is configured, cookies should be secure
if c.HTTP.TLS != nil {
return true
}
// If TLS is not configured, check if SecureSession is explicitly set in auth config
if c.HTTP.Auth != nil && c.HTTP.Auth.SecureSession != nil {
return *c.HTTP.Auth.SecureSession
}
// Default to false if TLS is not configured and no explicit setting
return false
}
// IsOpenIDSupported checks if the provider supports OpenID.
func IsOpenIDSupported(provider string) bool {
for _, supportedProvider := range openIDSupportedProviders {
pkg/api/config/config_test.go
+77
View File
@@ -1989,6 +1989,83 @@ func TestConfig(t *testing.T) {
So(cfg.IsMTLSAuthEnabled(), ShouldBeTrue) // No basic auth, so mTLS enabled
})
Convey("Test UseSecureSession()", func() {
// Test with nil Config
var cfg *config.Config = nil
So(cfg.UseSecureSession(), ShouldBeFalse)
// Test with Config but no TLS and no Auth
cfg = &config.Config{
HTTP: config.HTTPConfig{
TLS: nil,
Auth: nil,
},
}
So(cfg.UseSecureSession(), ShouldBeFalse)
// Test with TLS configured (should return true)
cfg = &config.Config{
HTTP: config.HTTPConfig{
TLS: &config.TLSConfig{
Cert: "/path/to/cert.pem",
Key: "/path/to/key.pem",
},
},
}
So(cfg.UseSecureSession(), ShouldBeTrue)
// Test with no TLS but SecureSession explicitly set to true
secureTrue := true
cfg = &config.Config{
HTTP: config.HTTPConfig{
TLS: nil,
Auth: &config.AuthConfig{
SecureSession: &secureTrue,
},
},
}
So(cfg.UseSecureSession(), ShouldBeTrue)
// Test with no TLS but SecureSession explicitly set to false
secureFalse := false
cfg = &config.Config{
HTTP: config.HTTPConfig{
TLS: nil,
Auth: &config.AuthConfig{
SecureSession: &secureFalse,
},
},
}
So(cfg.UseSecureSession(), ShouldBeFalse)
// Test with no TLS and Auth but SecureSession not set
cfg = &config.Config{
HTTP: config.HTTPConfig{
TLS: nil,
Auth: &config.AuthConfig{
APIKey: true,
},
},
}
So(cfg.UseSecureSession(), ShouldBeFalse)
// Test with TLS configured and SecureSession set (TLS should take precedence)
secureTrue = true
cfg = &config.Config{
HTTP: config.HTTPConfig{
TLS: &config.TLSConfig{
Cert: "/path/to/cert.pem",
Key: "/path/to/key.pem",
},
Auth: &config.AuthConfig{
SecureSession: &secureTrue,
},
},
}
So(cfg.UseSecureSession(), ShouldBeTrue) // TLS takes precedence
})
Convey("Test IsCompatEnabled()", func() {
// Test with nil Config
var cfg *config.Config = nil
pkg/api/controller_test.go
+5 -3
View File
@@ -5779,10 +5779,12 @@ func TestAuthnSessionErrors(t *testing.T) {
})
Convey("web request without cookies", func() {
client.SetCookie(&http.Cookie{})
// Create a new client without cookies to test unauthorized access
clientWithoutCookies := resty.New()
clientWithoutCookies.SetHeader(constants.SessionClientHeaderName, constants.SessionClientHeaderValue)
// call endpoint with session (added to client after previous request)
resp, err = client.R().
// call endpoint without session cookies
resp, err = clientWithoutCookies.R().
SetHeader(constants.SessionClientHeaderName, constants.SessionClientHeaderValue).
Get(baseURL + "/v2/_catalog")
So(err, ShouldBeNil)