From db7a1189699d9da8a60732202601ff9df49febe1 Mon Sep 17 00:00:00 2001 From: wojtekolesinski Date: Mon, 31 Jul 2023 15:56:12 +0200 Subject: [PATCH 1/6] change auth in logout --- server.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/server.go b/server.go index 37dc565f..38121d60 100644 --- a/server.go +++ b/server.go @@ -470,15 +470,17 @@ func (s *server) logout(w http.ResponseWriter, r *http.Request) { logger := common.RequestLogger(r, logModuleInfo) // Only header auth allowed for this endpoint - sessionID := common.GetBearerToken(r.Header.Get(s.authHeader)) - if sessionID == "" { - logger.Errorf("Request doesn't have a session value in header '%s'", s.authHeader) - w.WriteHeader(http.StatusUnauthorized) - return - } + //sessionID := common.GetBearerToken(r.Header.Get(s.authHeader)) + //if sessionID == "" { + // logger.Errorf("Request doesn't have a session value in header '%s'", s.authHeader) + // w.WriteHeader(http.StatusUnauthorized) + // return + //} + + //sessions.SessionFromRequest(r, s.store, sessions.UserSessionCookie, s.authHeader) // Revoke user session. - session, err := sessions.SessionFromID(sessionID, s.store) + session, _, err := sessions.SessionFromRequest(r, s.store, sessions.UserSessionCookie, s.authHeader) if err != nil { logger.Errorf("Couldn't get user session: %v", err) w.WriteHeader(http.StatusInternalServerError) From 65cea01951939359914c97263b77cc31149a66cc Mon Sep 17 00:00:00 2001 From: wojtekolesinski Date: Wed, 2 Aug 2023 15:51:39 +0200 Subject: [PATCH 2/6] remove dead code --- server.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/server.go b/server.go index 38121d60..f479570e 100644 --- a/server.go +++ b/server.go @@ -469,16 +469,6 @@ func (s *server) logout(w http.ResponseWriter, r *http.Request) { logger := common.RequestLogger(r, logModuleInfo) - // Only header auth allowed for this endpoint - //sessionID := common.GetBearerToken(r.Header.Get(s.authHeader)) - //if sessionID == "" { - // logger.Errorf("Request doesn't have a session value in header '%s'", s.authHeader) - // w.WriteHeader(http.StatusUnauthorized) - // return - //} - - //sessions.SessionFromRequest(r, s.store, sessions.UserSessionCookie, s.authHeader) - // Revoke user session. session, _, err := sessions.SessionFromRequest(r, s.store, sessions.UserSessionCookie, s.authHeader) if err != nil { From cb8ae53dae1d763d40b3a097ace356d1a05319e9 Mon Sep 17 00:00:00 2001 From: wojtekolesinski Date: Thu, 27 Jul 2023 11:49:44 +0200 Subject: [PATCH 3/6] server: Introduce support for HttpOnly and Secure attributes in session cookies --- README.md | 2 ++ common/settings.go | 46 ++++++++++++++++++++++++---------------------- main.go | 6 ++++-- server.go | 4 ++++ 4 files changed, 34 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index 1ed93206..badb93e7 100644 --- a/README.md +++ b/README.md @@ -100,6 +100,8 @@ Session store-related settings: | `OIDC_STATE_STORE_PATH` | "/var/lib/authservice/oidc_state.db" | Path to the session store used to save the sessions for the OIDC state parameter. | | `SESSION_MAX_AGE` | "86400" | Time in seconds after which sessions expire. Defaults to a day (24h). | | `SESSION_SAME_SITE` | "Lax" | SameSite attribute of the session cookie. Check details of SameSite attribute [here](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite). Its value can be "None", "Lax" or "Strict". | +| `SESSION_HTTP_ONLY` | `false` | HttpOnly attribute of the session cookie. Check details of SameSite attribute [here](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie#attributes). Its value can be "true" or "false". | +| `SESSION_SECURE` | `false` | Secure attribute of the session cookie. Check details of SameSite attribute [here](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie#attributes). Its value can be "true" or "false". | | `SESSION_STORE_TYPE`| "boltdb" | Set `SESSION_STORE_TYPE` to either "boltdb" to use BoltDB as the session store, or "redis" to use redis as the session store. Note that only one of the two can be used, also if you select redis then depending on your redis configurations then you might need to set the password and the number of the database that OIDC-AuthService will use as a [redis-client](https://redis.uptrace.dev/guide/go-redis.html#connecting-to-redis-server).| | `SESSION_STORE_REDIS_ADDR`| "127.0.0.1:6379" | Set the `host:port` address for the redis session store. | | `SESSION_STORE_REDIS_PWD`| "" | Set the password to connect with the redis session store. | diff --git a/common/settings.go b/common/settings.go index b1844ee7..53e9e12e 100644 --- a/common/settings.go +++ b/common/settings.go @@ -59,6 +59,8 @@ type Config struct { SessionStoreRedisDB int `split_words:"true" default:"0" envconfig:"SESSION_STORE_REDIS_DB"` SessionMaxAge int `split_words:"true" default:"86400"` SessionSameSite string `split_words:"true" default:"Lax"` + SessionHttpOnly bool `split_words:"true" default:"false"` + SessionSecure bool `split_words:"true" default:"false"` // Site ClientName string `split_words:"true" default:"AuthService"` @@ -102,17 +104,17 @@ func ParseConfig() (*Config, error) { if len(c.VerifyAuthURL.String()) == 0 { c.VerifyAuthURL = ResolvePathReference(c.AuthserviceURLPrefix, VerifyEndpoint) } - if !validAccessTokenAuthn(c.AccessTokenAuthnEnabled, c.AccessTokenAuthn){ - log.Fatalf("Unsupported access token authentication configuration:" + - "ACCESS_TOKEN_AUTHN=%s",c.AccessTokenAuthn) + if !validAccessTokenAuthn(c.AccessTokenAuthnEnabled, c.AccessTokenAuthn) { + log.Fatalf("Unsupported access token authentication configuration:"+ + "ACCESS_TOKEN_AUTHN=%s", c.AccessTokenAuthn) } - if !validSessionStoreType(c.SessionStoreType){ - log.Fatalf("Unsupported value for the type of the session store:" + - "SESSION_STORE_TYPE=%s",c.SessionStoreType) + if !validSessionStoreType(c.SessionStoreType) { + log.Fatalf("Unsupported value for the type of the session store:"+ + "SESSION_STORE_TYPE=%s", c.SessionStoreType) } - if !validLogLevel(c.LogLevel){ - log.Fatalf("Unsupported value for the log level messages:" + - "LOG_LEVEL=%s",c.LogLevel) + if !validLogLevel(c.LogLevel) { + log.Fatalf("Unsupported value for the log level messages:"+ + "LOG_LEVEL=%s", c.LogLevel) } c.UserTemplateContext = getEnvsFromPrefix("TEMPLATE_CONTEXT_") @@ -163,37 +165,37 @@ func ensureInSlice(elem string, slice []string) []string { // validAccessTokenAuthn() examines if the admins have configured // a valid value for the ACCESS_TOKEN_AUTHN envvar. -func validAccessTokenAuthn(AccessTokenAuthnEnabledEnv bool, AccessTokenAuthnEnv string) (bool){ +func validAccessTokenAuthn(AccessTokenAuthnEnabledEnv bool, AccessTokenAuthnEnv string) bool { if !AccessTokenAuthnEnabledEnv { return true } if AccessTokenAuthnEnv == "jwt" { return true } - if AccessTokenAuthnEnv == "opaque"{ + if AccessTokenAuthnEnv == "opaque" { return true } log.Warn("Please select exactly one of the supported options: " + - "i) jwt: to enable the JWT access token authentication method, " + - "ii) opaque: to enable the opaque access token authentication method") + "i) jwt: to enable the JWT access token authentication method, " + + "ii) opaque: to enable the opaque access token authentication method") return false } // validSessionStoreType() examines if the admins have configured a valid value // for the SESSION_STORE_TYPE envvar. -func validSessionStoreType(SessionStoreType string) (bool){ +func validSessionStoreType(SessionStoreType string) bool { if SessionStoreType == "boltdb" { return true } - if SessionStoreType == "redis"{ + if SessionStoreType == "redis" { return true } log.Warn("Please select exactly one of the options: " + - "i) boltdb: to select the BoltDB supported session store, " + - "ii) redis: to select the Redis supported session store") + "i) boltdb: to select the BoltDB supported session store, " + + "ii) redis: to select the Redis supported session store") return false } @@ -214,11 +216,11 @@ func validLogLevel(level string) bool { } log.Warn("Please select exactly one of the options for the LOG_LEVEL: " + - "i) FATAL: to print fatal log level messages, " + - "ii) ERROR: to print error log level messages and above, " + - "iii) WARN: to print warn log level messages and above, " + - "iv) INFO: to print info log level messages and above, " + - "v) DEBUG: to pring messages of all the available log levels") + "i) FATAL: to print fatal log level messages, " + + "ii) ERROR: to print error log level messages and above, " + + "iii) WARN: to print warn log level messages and above, " + + "iv) INFO: to print info log level messages and above, " + + "v) DEBUG: to pring messages of all the available log levels") return false } diff --git a/main.go b/main.go index 1e133237..bcbf9967 100644 --- a/main.go +++ b/main.go @@ -113,7 +113,7 @@ func main() { log.Fatalf("Error getting K8s config: %v", err) } else if err != nil { // If Kubernetes authenticator is disabled, ignore the error. - log.Debugf("Error getting K8s config: %v. " + + log.Debugf("Error getting K8s config: %v. "+ "Kubernetes authenticator is disabled, skipping ...", err) } else { k8sAuthenticator, err = authenticators.NewKubernetesAuthenticator( @@ -122,7 +122,7 @@ func main() { log.Fatalf("Error creating K8s authenticator: %v", err) } else if err != nil { // If Kubernetes authenticator is disabled, ignore the error. - log.Debugf("Error creating K8s authenticator:: %v. " + + log.Debugf("Error creating K8s authenticator:: %v. "+ "Kubernetes authenticator is disabled, skipping ...", err) } } @@ -218,6 +218,8 @@ func main() { }, userIdTransformer: c.UserIDTransformer, sessionMaxAgeSeconds: c.SessionMaxAge, + sessionHttpOnly: c.SessionHttpOnly, + sessionSecure: c.SessionSecure, strictSessionValidation: c.StrictSessionValidation, cacheEnabled: c.CacheEnabled, cacheExpirationMinutes: c.CacheExpirationMinutes, diff --git a/server.go b/server.go index f479570e..67175a42 100644 --- a/server.go +++ b/server.go @@ -66,6 +66,8 @@ type server struct { userIdTransformer common.UserIDTransformer caBundle []byte sessionSameSite http.SameSite + sessionHttpOnly bool + sessionSecure bool } // authenticate_or_login calls initiates the Authorization Code Flow if the user @@ -400,6 +402,8 @@ func (s *server) callback(w http.ResponseWriter, r *http.Request) { session.Options.Path = "/" // Extra layer of CSRF protection session.Options.SameSite = s.sessionSameSite + session.Options.HttpOnly = s.sessionHttpOnly + session.Options.Secure = s.sessionSecure userID, ok := claims[s.idTokenOpts.UserIDClaim].(string) if !ok { From 7ef11d41197b18a271fbd1aadfecf595e01851f6 Mon Sep 17 00:00:00 2001 From: wojtekolesinski Date: Wed, 2 Aug 2023 16:03:58 +0200 Subject: [PATCH 4/6] redo automatic reformat --- main.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/main.go b/main.go index bcbf9967..69b22ed2 100644 --- a/main.go +++ b/main.go @@ -113,7 +113,7 @@ func main() { log.Fatalf("Error getting K8s config: %v", err) } else if err != nil { // If Kubernetes authenticator is disabled, ignore the error. - log.Debugf("Error getting K8s config: %v. "+ + log.Debugf("Error getting K8s config: %v. " + "Kubernetes authenticator is disabled, skipping ...", err) } else { k8sAuthenticator, err = authenticators.NewKubernetesAuthenticator( @@ -122,7 +122,7 @@ func main() { log.Fatalf("Error creating K8s authenticator: %v", err) } else if err != nil { // If Kubernetes authenticator is disabled, ignore the error. - log.Debugf("Error creating K8s authenticator:: %v. "+ + log.Debugf("Error creating K8s authenticator:: %v. " + "Kubernetes authenticator is disabled, skipping ...", err) } } From 1d53e9777dd6f39e85fb615c9b3a1153e97f908c Mon Sep 17 00:00:00 2001 From: wojtekolesinski Date: Wed, 2 Aug 2023 16:05:44 +0200 Subject: [PATCH 5/6] redo automatic reformat --- common/settings.go | 44 ++++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/common/settings.go b/common/settings.go index 53e9e12e..b0e91444 100644 --- a/common/settings.go +++ b/common/settings.go @@ -104,17 +104,17 @@ func ParseConfig() (*Config, error) { if len(c.VerifyAuthURL.String()) == 0 { c.VerifyAuthURL = ResolvePathReference(c.AuthserviceURLPrefix, VerifyEndpoint) } - if !validAccessTokenAuthn(c.AccessTokenAuthnEnabled, c.AccessTokenAuthn) { - log.Fatalf("Unsupported access token authentication configuration:"+ - "ACCESS_TOKEN_AUTHN=%s", c.AccessTokenAuthn) + if !validAccessTokenAuthn(c.AccessTokenAuthnEnabled, c.AccessTokenAuthn){ + log.Fatalf("Unsupported access token authentication configuration:" + + "ACCESS_TOKEN_AUTHN=%s",c.AccessTokenAuthn) } - if !validSessionStoreType(c.SessionStoreType) { - log.Fatalf("Unsupported value for the type of the session store:"+ - "SESSION_STORE_TYPE=%s", c.SessionStoreType) + if !validSessionStoreType(c.SessionStoreType){ + log.Fatalf("Unsupported value for the type of the session store:" + + "SESSION_STORE_TYPE=%s",c.SessionStoreType) } - if !validLogLevel(c.LogLevel) { - log.Fatalf("Unsupported value for the log level messages:"+ - "LOG_LEVEL=%s", c.LogLevel) + if !validLogLevel(c.LogLevel){ + log.Fatalf("Unsupported value for the log level messages:" + + "LOG_LEVEL=%s",c.LogLevel) } c.UserTemplateContext = getEnvsFromPrefix("TEMPLATE_CONTEXT_") @@ -165,37 +165,37 @@ func ensureInSlice(elem string, slice []string) []string { // validAccessTokenAuthn() examines if the admins have configured // a valid value for the ACCESS_TOKEN_AUTHN envvar. -func validAccessTokenAuthn(AccessTokenAuthnEnabledEnv bool, AccessTokenAuthnEnv string) bool { +func validAccessTokenAuthn(AccessTokenAuthnEnabledEnv bool, AccessTokenAuthnEnv string) (bool){ if !AccessTokenAuthnEnabledEnv { return true } if AccessTokenAuthnEnv == "jwt" { return true } - if AccessTokenAuthnEnv == "opaque" { + if AccessTokenAuthnEnv == "opaque"{ return true } log.Warn("Please select exactly one of the supported options: " + - "i) jwt: to enable the JWT access token authentication method, " + - "ii) opaque: to enable the opaque access token authentication method") + "i) jwt: to enable the JWT access token authentication method, " + + "ii) opaque: to enable the opaque access token authentication method") return false } // validSessionStoreType() examines if the admins have configured a valid value // for the SESSION_STORE_TYPE envvar. -func validSessionStoreType(SessionStoreType string) bool { +func validSessionStoreType(SessionStoreType string) (bool){ if SessionStoreType == "boltdb" { return true } - if SessionStoreType == "redis" { + if SessionStoreType == "redis"{ return true } log.Warn("Please select exactly one of the options: " + - "i) boltdb: to select the BoltDB supported session store, " + - "ii) redis: to select the Redis supported session store") + "i) boltdb: to select the BoltDB supported session store, " + + "ii) redis: to select the Redis supported session store") return false } @@ -216,11 +216,11 @@ func validLogLevel(level string) bool { } log.Warn("Please select exactly one of the options for the LOG_LEVEL: " + - "i) FATAL: to print fatal log level messages, " + - "ii) ERROR: to print error log level messages and above, " + - "iii) WARN: to print warn log level messages and above, " + - "iv) INFO: to print info log level messages and above, " + - "v) DEBUG: to pring messages of all the available log levels") + "i) FATAL: to print fatal log level messages, " + + "ii) ERROR: to print error log level messages and above, " + + "iii) WARN: to print warn log level messages and above, " + + "iv) INFO: to print info log level messages and above, " + + "v) DEBUG: to pring messages of all the available log levels") return false } From ca8ca3ea9a14dd353a7943a872a3ca1a91e06ad4 Mon Sep 17 00:00:00 2001 From: wojtekolesinski Date: Wed, 2 Aug 2023 16:37:47 +0200 Subject: [PATCH 6/6] change logout flow --- server.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server.go b/server.go index 67175a42..0777c252 100644 --- a/server.go +++ b/server.go @@ -474,13 +474,13 @@ func (s *server) logout(w http.ResponseWriter, r *http.Request) { logger := common.RequestLogger(r, logModuleInfo) // Revoke user session. - session, _, err := sessions.SessionFromRequest(r, s.store, sessions.UserSessionCookie, s.authHeader) + session, authMethod, err := sessions.SessionFromRequest(r, s.store, sessions.UserSessionCookie, s.authHeader) if err != nil { logger.Errorf("Couldn't get user session: %v", err) w.WriteHeader(http.StatusInternalServerError) return } - if session.IsNew { + if authMethod == "" { logger.Warn("Request doesn't have a valid session.") w.WriteHeader(http.StatusUnauthorized) return