Refactor cookie, fix cookie invalidation

p53 · Nov 15, 2023 · 020c911 · 020c911
1 parent ce8502d
commit 020c911
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 11 deletions.
diff --git a/pkg/constant/constant.go b/pkg/constant/constant.go
@@ -1,5 +1,7 @@
 package constant
 
+import "time"
+
 type contextKey int8
 
 const (
@@ -65,4 +67,6 @@ const (
 	IdpRevokeURI      = "/protocol/openid-connect/revoke"
 	IdpResourceSetURI = "/authz/protection/resource_set"
 	IdpProtectPermURI = "/authz/protection/permission"
+
+	InvalidCookieDuration = -10 * time.Hour
 )
diff --git a/pkg/keycloak/proxy/handlers.go b/pkg/keycloak/proxy/handlers.go
@@ -100,7 +100,6 @@ func (r *OauthProxy) oauthAuthorizationHandler(wrt http.ResponseWriter, req *htt
 	}
 
 	scope, assertOk := req.Context().Value(constant.ContextScopeName).(*RequestScope)
-
 	if !assertOk {
 		r.Log.Error(apperrors.ErrAssertionFailed.Error())
 		return
@@ -122,7 +121,6 @@ func (r *OauthProxy) oauthAuthorizationHandler(wrt http.ResponseWriter, req *htt
 
 	if r.Config.EnablePKCE {
 		codeVerifier, err := pkce.NewCodeVerifierWithLength(96)
-
 		if err != nil {
 			r.Log.Error(
 				apperrors.ErrPKCECodeCreation.Error(),
@@ -136,7 +134,7 @@ func (r *OauthProxy) oauthAuthorizationHandler(wrt http.ResponseWriter, req *htt
 			oauth2.SetAuthURLParam(pkce.ParamCodeChallenge, codeChallenge),
 			oauth2.SetAuthURLParam(pkce.ParamCodeChallengeMethod, pkce.MethodS256),
 		)
-		r.Cm.WritePKCECookie(wrt, codeVerifier)
+		r.Cm.DropPKCECookie(wrt, codeVerifier)
 	}
 
 	authURL := conf.AuthCodeURL(
@@ -238,6 +236,9 @@ func (r *OauthProxy) oauthCallbackHandler(writer http.ResponseWriter, req *http.
 		}
 	}
 
+	r.Cm.ClearStateParameterCookie(req, writer)
+	r.Cm.ClearPKCECookie(req, writer)
+
 	if r.Config.PostLoginRedirectPath != "" && redirectURI == "/" {
 		redirectURI = r.Config.PostLoginRedirectPath
 	}

diff --git a/pkg/keycloak/proxy/misc.go b/pkg/keycloak/proxy/misc.go
@@ -164,7 +164,7 @@ func (r *OauthProxy) redirectToAuthorization(wrt http.ResponseWriter, req *http.
 	}
 
 	// step: add a state referrer to the authorization page
-	uuid := r.Cm.WriteStateParameterCookie(req, wrt)
+	uuid := r.Cm.DropStateParameterCookie(req, wrt)
 	authQuery := fmt.Sprintf("?state=%s", uuid)
 
 	// step: if verification is switched off, we can't authorization

diff --git a/pkg/proxy/cookie/cookies.go b/pkg/proxy/cookie/cookies.go
@@ -71,7 +71,7 @@ func (cm *Manager) DropCookie(
 		Value:    value,
 	}
 
-	if !cm.EnableSessionCookies && duration != 0 {
+	if !cm.EnableSessionCookies && duration != 0 || duration == constant.InvalidCookieDuration {
 		cookie.Expires = time.Now().Add(duration)
 	}
 
@@ -88,6 +88,7 @@ func (cm *Manager) DropCookie(
 }
 
 // maxCookieChunkSize calculates max cookie chunk size, which can be used for cookie value
+// this seems to be not useful as many browsers have limits of all cookies per domain = 4096 bytes
 func (cm *Manager) GetMaxCookieChunkLength(
 	req *http.Request,
 	cookieName string,
@@ -176,8 +177,8 @@ func (cm *Manager) DropUMATokenCookie(req *http.Request, w http.ResponseWriter,
 	cm.dropCookieWithChunks(req, w, cm.CookieUMAName, value, duration)
 }
 
-// writeStateParameterCookie sets a state parameter cookie into the response
-func (cm *Manager) WriteStateParameterCookie(req *http.Request, wrt http.ResponseWriter) string {
+// DropStateParameterCookie sets a state parameter cookie into the response
+func (cm *Manager) DropStateParameterCookie(req *http.Request, wrt http.ResponseWriter) string {
 	uuid, err := uuid.NewV4()
 
 	if err != nil {
@@ -199,8 +200,8 @@ func (cm *Manager) WriteStateParameterCookie(req *http.Request, wrt http.Respons
 	return uuid.String()
 }
 
-// writePKCECookie sets a code verifier cookie into the response
-func (cm *Manager) WritePKCECookie(wrt http.ResponseWriter, codeVerifier string) {
+// DropPKCECookie sets a code verifier cookie into the response
+func (cm *Manager) DropPKCECookie(wrt http.ResponseWriter, codeVerifier string) {
 	cm.DropCookie(wrt, cm.CookiePKCEName, codeVerifier, 0)
 }
 
@@ -210,10 +211,11 @@ func (cm *Manager) ClearAllCookies(req *http.Request, w http.ResponseWriter) {
 	cm.ClearRefreshTokenCookie(req, w)
 	cm.ClearIDTokenCookie(req, w)
 	cm.ClearUMATokenCookie(req, w)
+	cm.ClearStateParameterCookie(req, w)
 }
 
 func (cm *Manager) ClearCookie(req *http.Request, wrt http.ResponseWriter, name string) {
-	cm.DropCookie(wrt, name, "", -10*time.Hour)
+	cm.DropCookie(wrt, name, "", constant.InvalidCookieDuration)
 
 	// clear divided cookies
 	for idx := 1; idx < 600; idx++ {
@@ -224,7 +226,7 @@ func (cm *Manager) ClearCookie(req *http.Request, wrt http.ResponseWriter, name
 				wrt,
 				name+"-"+strconv.Itoa(idx),
 				"",
-				-10*time.Hour,
+				constant.InvalidCookieDuration,
 			)
 		} else {
 			break
@@ -251,3 +253,14 @@ func (cm *Manager) ClearIDTokenCookie(req *http.Request, wrt http.ResponseWriter
 func (cm *Manager) ClearUMATokenCookie(req *http.Request, wrt http.ResponseWriter) {
 	cm.ClearCookie(req, wrt, cm.CookieUMAName)
 }
+
+// ClearPKCECookie clears the session cookie
+func (cm *Manager) ClearPKCECookie(req *http.Request, wrt http.ResponseWriter) {
+	cm.ClearCookie(req, wrt, cm.CookiePKCEName)
+}
+
+// ClearStateParameterCookie clears the session cookie
+func (cm *Manager) ClearStateParameterCookie(req *http.Request, wrt http.ResponseWriter) {
+	cm.ClearCookie(req, wrt, cm.CookieRequestURIName)
+	cm.ClearCookie(req, wrt, cm.CookieOAuthStateName)
+}