feat: make session refresh timeouts user configurable
Make the session refresh lock duration, obtain timeout, and retry period configurable via command-line flags, config file, or environment variables. Previously, these values were hardcoded constants: - sessionRefreshLockDuration = 2s - sessionRefreshObtainTimeout = 5s - sessionRefreshRetryPeriod = 10ms This change removes the hardcoded constants and adds three new configuration options to the Cookie configuration: - `--session-refresh-lock-duration` (default: 2s) - `--session-refresh-obtain-timeout` (default: 5s) - `--session-refresh-retry-period` (default: 10ms) The new options allow users to tune session refresh behavior for their specific deployment requirements, such as: - High-latency networks requiring longer timeouts - High-throughput systems needing faster retry intervals - Provider-specific refresh operation durations Changes: - Added SessionRefreshLockDuration, SessionRefreshObtainTimeout, and SessionRefreshRetryPeriod fields to Cookie struct - Updated StoredSessionLoaderOptions to accept these as parameters - Modified storedSessionLoader to use configurable values instead of constants - Updated all tests to provide default values - Maintains full backward compatibility with original default values Resolves TODO comments in pkg/middleware/stored_session.go Signed-off-by: Kinfemichael Desse <kinfemichael.desse@real-digital.de>
This commit is contained in:
Kinfemichael Desse
parent
3a55dadbe8
commit
70be1b2893
|
|
@ -415,10 +415,13 @@ func buildSessionChain(opts *options.Options, provider providers.Provider, sessi
|
|||
}
|
||||
|
||||
chain = chain.Append(middleware.NewStoredSessionLoader(&middleware.StoredSessionLoaderOptions{
|
||||
SessionStore: sessionStore,
|
||||
RefreshPeriod: opts.Cookie.Refresh,
|
||||
RefreshSession: provider.RefreshSession,
|
||||
ValidateSession: provider.ValidateSession,
|
||||
SessionStore: sessionStore,
|
||||
RefreshPeriod: opts.Cookie.Refresh,
|
||||
SessionRefreshLockDuration: opts.Cookie.SessionRefreshLockDuration,
|
||||
SessionRefreshObtainTimeout: opts.Cookie.SessionRefreshObtainTimeout,
|
||||
SessionRefreshRetryPeriod: opts.Cookie.SessionRefreshRetryPeriod,
|
||||
RefreshSession: provider.RefreshSession,
|
||||
ValidateSession: provider.ValidateSession,
|
||||
}))
|
||||
|
||||
return chain
|
||||
|
|
|
|||
|
|
@ -11,19 +11,22 @@ import (
|
|||
|
||||
// Cookie contains configuration options relating to Cookie configuration
|
||||
type Cookie struct {
|
||||
Name string `flag:"cookie-name" cfg:"cookie_name"`
|
||||
Secret string `flag:"cookie-secret" cfg:"cookie_secret"`
|
||||
SecretFile string `flag:"cookie-secret-file" cfg:"cookie_secret_file"`
|
||||
Domains []string `flag:"cookie-domain" cfg:"cookie_domains"`
|
||||
Path string `flag:"cookie-path" cfg:"cookie_path"`
|
||||
Expire time.Duration `flag:"cookie-expire" cfg:"cookie_expire"`
|
||||
Refresh time.Duration `flag:"cookie-refresh" cfg:"cookie_refresh"`
|
||||
Secure bool `flag:"cookie-secure" cfg:"cookie_secure"`
|
||||
HTTPOnly bool `flag:"cookie-httponly" cfg:"cookie_httponly"`
|
||||
SameSite string `flag:"cookie-samesite" cfg:"cookie_samesite"`
|
||||
CSRFPerRequest bool `flag:"cookie-csrf-per-request" cfg:"cookie_csrf_per_request"`
|
||||
CSRFPerRequestLimit int `flag:"cookie-csrf-per-request-limit" cfg:"cookie_csrf_per_request_limit"`
|
||||
CSRFExpire time.Duration `flag:"cookie-csrf-expire" cfg:"cookie_csrf_expire"`
|
||||
Name string `flag:"cookie-name" cfg:"cookie_name"`
|
||||
Secret string `flag:"cookie-secret" cfg:"cookie_secret"`
|
||||
SecretFile string `flag:"cookie-secret-file" cfg:"cookie_secret_file"`
|
||||
Domains []string `flag:"cookie-domain" cfg:"cookie_domains"`
|
||||
Path string `flag:"cookie-path" cfg:"cookie_path"`
|
||||
Expire time.Duration `flag:"cookie-expire" cfg:"cookie_expire"`
|
||||
Refresh time.Duration `flag:"cookie-refresh" cfg:"cookie_refresh"`
|
||||
Secure bool `flag:"cookie-secure" cfg:"cookie_secure"`
|
||||
HTTPOnly bool `flag:"cookie-httponly" cfg:"cookie_httponly"`
|
||||
SameSite string `flag:"cookie-samesite" cfg:"cookie_samesite"`
|
||||
CSRFPerRequest bool `flag:"cookie-csrf-per-request" cfg:"cookie_csrf_per_request"`
|
||||
CSRFPerRequestLimit int `flag:"cookie-csrf-per-request-limit" cfg:"cookie_csrf_per_request_limit"`
|
||||
CSRFExpire time.Duration `flag:"cookie-csrf-expire" cfg:"cookie_csrf_expire"`
|
||||
SessionRefreshLockDuration time.Duration `flag:"session-refresh-lock-duration" cfg:"session_refresh_lock_duration"`
|
||||
SessionRefreshObtainTimeout time.Duration `flag:"session-refresh-obtain-timeout" cfg:"session_refresh_obtain_timeout"`
|
||||
SessionRefreshRetryPeriod time.Duration `flag:"session-refresh-retry-period" cfg:"session_refresh_retry_period"`
|
||||
}
|
||||
|
||||
func cookieFlagSet() *pflag.FlagSet {
|
||||
|
|
@ -42,25 +45,31 @@ func cookieFlagSet() *pflag.FlagSet {
|
|||
flagSet.Bool("cookie-csrf-per-request", false, "When this property is set to true, then the CSRF cookie name is built based on the state and varies per request. If property is set to false, then CSRF cookie has the same name for all requests.")
|
||||
flagSet.Int("cookie-csrf-per-request-limit", 0, "Sets a limit on the number of CSRF requests cookies that oauth2-proxy will create. The oldest cookies will be removed. Useful if users end up with 431 Request headers too large status codes.")
|
||||
flagSet.Duration("cookie-csrf-expire", time.Duration(15)*time.Minute, "expire timeframe for CSRF cookie")
|
||||
flagSet.Duration("session-refresh-lock-duration", time.Duration(2)*time.Second, "maximum time allowed for a session refresh attempt; if the refresh request isn't finished within this time, the lock will be released")
|
||||
flagSet.Duration("session-refresh-obtain-timeout", time.Duration(5)*time.Second, "timeout when attempting to obtain the session lock; if the lock is not obtained before this timeout, the refresh attempt will fail")
|
||||
flagSet.Duration("session-refresh-retry-period", time.Duration(10)*time.Millisecond, "how long to wait after failing to obtain the lock before trying again")
|
||||
return flagSet
|
||||
}
|
||||
|
||||
// cookieDefaults creates a Cookie populating each field with its default value
|
||||
func cookieDefaults() Cookie {
|
||||
return Cookie{
|
||||
Name: "_oauth2_proxy",
|
||||
Secret: "",
|
||||
SecretFile: "",
|
||||
Domains: nil,
|
||||
Path: "/",
|
||||
Expire: time.Duration(168) * time.Hour,
|
||||
Refresh: time.Duration(0),
|
||||
Secure: true,
|
||||
HTTPOnly: true,
|
||||
SameSite: "",
|
||||
CSRFPerRequest: false,
|
||||
CSRFPerRequestLimit: 0,
|
||||
CSRFExpire: time.Duration(15) * time.Minute,
|
||||
Name: "_oauth2_proxy",
|
||||
Secret: "",
|
||||
SecretFile: "",
|
||||
Domains: nil,
|
||||
Path: "/",
|
||||
Expire: time.Duration(168) * time.Hour,
|
||||
Refresh: time.Duration(0),
|
||||
Secure: true,
|
||||
HTTPOnly: true,
|
||||
SameSite: "",
|
||||
CSRFPerRequest: false,
|
||||
CSRFPerRequestLimit: 0,
|
||||
CSRFExpire: time.Duration(15) * time.Minute,
|
||||
SessionRefreshLockDuration: time.Duration(2) * time.Second,
|
||||
SessionRefreshObtainTimeout: time.Duration(5) * time.Second,
|
||||
SessionRefreshRetryPeriod: time.Duration(10) * time.Millisecond,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -14,23 +14,6 @@ import (
|
|||
"github.com/oauth2-proxy/oauth2-proxy/v7/providers"
|
||||
)
|
||||
|
||||
const (
|
||||
// When attempting to obtain the lock, if it's not done before this timeout
|
||||
// then exit and fail the refresh attempt.
|
||||
// TODO: This should probably be configurable by the end user.
|
||||
sessionRefreshObtainTimeout = 5 * time.Second
|
||||
|
||||
// Maximum time allowed for a session refresh attempt.
|
||||
// If the refresh request isn't finished within this time, the lock will be
|
||||
// released.
|
||||
// TODO: This should probably be configurable by the end user.
|
||||
sessionRefreshLockDuration = 2 * time.Second
|
||||
|
||||
// How long to wait after failing to obtain the lock before trying again.
|
||||
// TODO: This should probably be configurable by the end user.
|
||||
sessionRefreshRetryPeriod = 10 * time.Millisecond
|
||||
)
|
||||
|
||||
// StoredSessionLoaderOptions contains all of the requirements to construct
|
||||
// a stored session loader.
|
||||
// All options must be provided.
|
||||
|
|
@ -41,6 +24,17 @@ type StoredSessionLoaderOptions struct {
|
|||
// How often should sessions be refreshed
|
||||
RefreshPeriod time.Duration
|
||||
|
||||
// Maximum time allowed for a session refresh attempt.
|
||||
// If the refresh request isn't finished within this time, the lock will be released.
|
||||
SessionRefreshLockDuration time.Duration
|
||||
|
||||
// Timeout when attempting to obtain the session lock.
|
||||
// If the lock is not obtained before this timeout, the refresh attempt will fail.
|
||||
SessionRefreshObtainTimeout time.Duration
|
||||
|
||||
// How long to wait after failing to obtain the lock before trying again.
|
||||
SessionRefreshRetryPeriod time.Duration
|
||||
|
||||
// Provider based session refreshing
|
||||
RefreshSession func(context.Context, *sessionsapi.SessionState) (bool, error)
|
||||
|
||||
|
|
@ -56,10 +50,13 @@ type StoredSessionLoaderOptions struct {
|
|||
// If a session was loader by a previous handler, it will not be replaced.
|
||||
func NewStoredSessionLoader(opts *StoredSessionLoaderOptions) alice.Constructor {
|
||||
ss := &storedSessionLoader{
|
||||
store: opts.SessionStore,
|
||||
refreshPeriod: opts.RefreshPeriod,
|
||||
sessionRefresher: opts.RefreshSession,
|
||||
sessionValidator: opts.ValidateSession,
|
||||
store: opts.SessionStore,
|
||||
refreshPeriod: opts.RefreshPeriod,
|
||||
sessionRefreshLockDuration: opts.SessionRefreshLockDuration,
|
||||
sessionRefreshObtainTimeout: opts.SessionRefreshObtainTimeout,
|
||||
sessionRefreshRetryPeriod: opts.SessionRefreshRetryPeriod,
|
||||
sessionRefresher: opts.RefreshSession,
|
||||
sessionValidator: opts.ValidateSession,
|
||||
}
|
||||
return ss.loadSession
|
||||
}
|
||||
|
|
@ -67,10 +64,13 @@ func NewStoredSessionLoader(opts *StoredSessionLoaderOptions) alice.Constructor
|
|||
// storedSessionLoader is responsible for loading sessions from cookie
|
||||
// identified sessions in the session store.
|
||||
type storedSessionLoader struct {
|
||||
store sessionsapi.SessionStore
|
||||
refreshPeriod time.Duration
|
||||
sessionRefresher func(context.Context, *sessionsapi.SessionState) (bool, error)
|
||||
sessionValidator func(context.Context, *sessionsapi.SessionState) bool
|
||||
store sessionsapi.SessionStore
|
||||
refreshPeriod time.Duration
|
||||
sessionRefreshLockDuration time.Duration
|
||||
sessionRefreshObtainTimeout time.Duration
|
||||
sessionRefreshRetryPeriod time.Duration
|
||||
sessionRefresher func(context.Context, *sessionsapi.SessionState) (bool, error)
|
||||
sessionValidator func(context.Context, *sessionsapi.SessionState) bool
|
||||
}
|
||||
|
||||
// loadSession attempts to load a session as identified by the request cookies.
|
||||
|
|
@ -131,7 +131,7 @@ func (s *storedSessionLoader) refreshSessionIfNeeded(rw http.ResponseWriter, req
|
|||
}
|
||||
|
||||
var lockObtained bool
|
||||
ctx, cancel := context.WithTimeout(context.Background(), sessionRefreshObtainTimeout)
|
||||
ctx, cancel := context.WithTimeout(context.Background(), s.sessionRefreshObtainTimeout)
|
||||
defer cancel()
|
||||
|
||||
for !lockObtained {
|
||||
|
|
@ -139,11 +139,11 @@ func (s *storedSessionLoader) refreshSessionIfNeeded(rw http.ResponseWriter, req
|
|||
case <-ctx.Done():
|
||||
return errors.New("timeout obtaining session lock")
|
||||
default:
|
||||
err := session.ObtainLock(req.Context(), sessionRefreshLockDuration)
|
||||
err := session.ObtainLock(req.Context(), s.sessionRefreshLockDuration)
|
||||
if err != nil && !errors.Is(err, sessionsapi.ErrLockNotObtained) {
|
||||
return fmt.Errorf("error occurred while trying to obtain lock: %v", err)
|
||||
} else if errors.Is(err, sessionsapi.ErrLockNotObtained) {
|
||||
time.Sleep(sessionRefreshRetryPeriod)
|
||||
time.Sleep(s.sessionRefreshRetryPeriod)
|
||||
continue
|
||||
}
|
||||
// No error means we obtained the lock
|
||||
|
|
|
|||
|
|
@ -183,10 +183,13 @@ var _ = Describe("Stored Session Suite", func() {
|
|||
rw := httptest.NewRecorder()
|
||||
|
||||
opts := &StoredSessionLoaderOptions{
|
||||
SessionStore: in.store,
|
||||
RefreshPeriod: in.refreshPeriod,
|
||||
RefreshSession: in.refreshSession,
|
||||
ValidateSession: in.validateSession,
|
||||
SessionStore: in.store,
|
||||
RefreshPeriod: in.refreshPeriod,
|
||||
SessionRefreshLockDuration: 2 * time.Second,
|
||||
SessionRefreshObtainTimeout: 5 * time.Second,
|
||||
SessionRefreshRetryPeriod: 10 * time.Millisecond,
|
||||
RefreshSession: in.refreshSession,
|
||||
ValidateSession: in.validateSession,
|
||||
}
|
||||
|
||||
// Create the handler with a next handler that will capture the session
|
||||
|
|
@ -383,8 +386,11 @@ var _ = Describe("Stored Session Suite", func() {
|
|||
|
||||
sessionRefreshed := false
|
||||
opts := &StoredSessionLoaderOptions{
|
||||
SessionStore: store,
|
||||
RefreshPeriod: in.refreshPeriod,
|
||||
SessionStore: store,
|
||||
RefreshPeriod: in.refreshPeriod,
|
||||
SessionRefreshLockDuration: 2 * time.Second,
|
||||
SessionRefreshObtainTimeout: 5 * time.Second,
|
||||
SessionRefreshRetryPeriod: 10 * time.Millisecond,
|
||||
RefreshSession: func(ctx context.Context, s *sessionsapi.SessionState) (bool, error) {
|
||||
time.Sleep(10 * time.Millisecond)
|
||||
sessionRefreshed = true
|
||||
|
|
@ -479,8 +485,11 @@ var _ = Describe("Stored Session Suite", func() {
|
|||
}
|
||||
|
||||
s := &storedSessionLoader{
|
||||
refreshPeriod: in.refreshPeriod,
|
||||
store: store,
|
||||
refreshPeriod: in.refreshPeriod,
|
||||
store: store,
|
||||
sessionRefreshLockDuration: 2 * time.Second,
|
||||
sessionRefreshObtainTimeout: 5 * time.Second,
|
||||
sessionRefreshRetryPeriod: 10 * time.Millisecond,
|
||||
sessionRefresher: func(_ context.Context, ss *sessionsapi.SessionState) (bool, error) {
|
||||
refreshed = true
|
||||
switch ss.RefreshToken {
|
||||
|
|
|
|||
Loading…
Reference in New Issue